feat: add root project import in import_map

[xstevenyung]

[lucacasonato]

Can you also change the code of the initialized project to make use of @/?

[xstevenyung]

good call.

updated 👍

[digitaldesigndj]

I ❤️ "@/": "./"

Already using this on my projects. Would love to see it become standard. Only confused about the new preact-signals import on fresh 1.1.0 why is it prefixed with @?

[xstevenyung]

It seems like there is some issue with this change on Windows. I think this is unrelated of this PR

[thesobercoder]

@xstevenyung I created a new project and added "@/": "./". It works perfectly on Windows!

[iuioiua]

Tested and working on macOS! I do believe that this change causes the issue on Windows. Is someone with a Windows machine able to troubleshoot?

[iuioiua]

Tested and working on macOS! I do believe that this change causes the issue on Windows. Is someone with a Windows machine able to troubleshoot?

@ayame113, any chance you'd be able to look at this?

[ayame113]

I will look into this later! please wait a little bit!

[ayame113]

Unfortunately I can't reproduce it locally. Can you try rerunning the test and see if this happens again?

[iuioiua]

After some debugging, it appears when import Counter from "../islands/Counter.tsx"; is replaced with import Counter from "@/islands/Counter.tsx"; in /init.ts, CI fails on Windows. I'm not sure why.

[ayame113]

I've been debugging this for several days to get to the cause of the CI failure. I think I may have figured out the cause. I'd like to create a minimal repro example and then post the details here.

[ayame113]

This is due to a 20 year old Windows backwards compatibility feature and a Deno CLI bug related to it.

Deno CLI bug for short file names

Windows has a feature called "short file names" for backward compatibility with Windows 95. This is a feature that creates short 8-character aliases for long paths or paths with spaces.

https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-2000-server/cc976806(v=technet.10)

# Alias for `test dir` is `TESTDI~1`.
C:\Users\ayame\work>dir /x
2023/05/26  15:32    <DIR>                       .
2023/05/09  08:52    <DIR>                       ..
2023/05/26  15:35    <DIR>          TESTDI~1     test dir

Deno CLI has a bug related to short file names. This only happens when combining deno.json, import-maps with relative paths, and short file names.

The following example works fine.

C:\Users\ayame\work>cd "./test dir"
C:\Users\ayame\work\test dir>type import-map.json
{
  "imports": {
    "@/": "./"
  }
}
C:\Users\ayame\work\test dir>type parent.ts
import "@/child.ts";
import "./child.ts";
C:\Users\ayame\work\test dir>type child.ts
console.log("hello from child.ts!");
C:\Users\ayame\work\test dir>deno info --import-map=import-map.json ./parent.ts
local: C:\Users\ayame\work\test dir\parent.ts
emit: C:\Users\ayame\AppData\Local\deno\gen\file\C\Users\ayame\work\test dir\parent.ts.js
type: TypeScript
dependencies: 1 unique
size: 82B

file:///C:/Users/ayame/work/test%20dir/parent.ts (44B)
├── file:///C:/Users/ayame/work/test%20dir/child.ts (38B)
└── file:///C:/Users/ayame/work/test%20dir/child.ts *
C:\Users\ayame\work\test dir>deno run --import-map=import-map.json ./parent.ts
hello from child.ts!

Next, move the current directory to the directory with the short file name. This also still works fine.

C:\Users\ayame\work\test dir>cd ../TESTDI~1
C:\Users\ayame\work\TESTDI~1>dir

 C:\Users\ayame\work\TESTDI~1

2023/05/26  15:44    <DIR>          .
2023/05/26  15:32    <DIR>          ..
2023/05/26  15:32                38 child.ts
2023/05/26  03:15                43 import-map.json
2023/05/26  03:20                44 parent.ts
C:\Users\ayame\work\TESTDI~1>deno info --import-map=import-map.json ./parent.ts
local: C:\Users\ayame\work\TESTDI~1\parent.ts
emit: C:\Users\ayame\AppData\Local\deno\gen\file\C\Users\ayame\work\TESTDI~1\parent.ts.js
type: TypeScript
dependencies: 1 unique
size: 82B

file:///C:/Users/ayame/work/TESTDI~1/parent.ts (44B)
├── file:///C:/Users/ayame/work/TESTDI~1/child.ts (38B)
└── file:///C:/Users/ayame/work/TESTDI~1/child.ts *
C:\Users\ayame\work\TESTDI~1>deno run --import-map=import-map.json ./parent.ts
hello from child.ts!

Now, creating deno.json messes up dependency resolution. child.ts is loaded twice. This is the bug.

C:\Users\ayame\work\TESTDI~1>type deno.json
{
  "importMap": "./import-map.json"
}
# ↓`test%20dir/child.ts` and `TESTDI~1/child.ts` are included in dependencies.
C:\Users\ayame\work\TESTDI~1>deno info ./parent.ts
local: C:\Users\ayame\work\TESTDI~1\parent.ts
emit: C:\Users\ayame\AppData\Local\deno\gen\file\C\Users\ayame\work\TESTDI~1\parent.ts.js
type: TypeScript
dependencies: 2 unique
size: 120B

file:///C:/Users/ayame/work/TESTDI~1/parent.ts (44B)
├── file:///C:/Users/ayame/work/test%20dir/child.ts (38B)
└── file:///C:/Users/ayame/work/TESTDI~1/child.ts (38B)
# ↓The hello message will be displayed twice.(bug)
C:\Users\ayame\work\TESTDI~1>deno run ./parent.ts
hello from child.ts!
hello from child.ts!

Temporary directory in GitHub Actions

The temporary directory in GitHub Actions is set to short file name by default. It should be C:\Users\runneradmin\AppData\Local\Temp, but the actual value is C:\Users\RUNNER~1\AppData\Local\Temp. (This is exactly the phenomenon described in https://github.com/actions/runner-images/issues/712.)

Fresh's tests/cli_test.ts uses this temporary directory. It will run init.ts inside the temporary directory and create a fresh project inside.

https://github.com/denoland/fresh/blob/7f3fd51cca75797ce9bf6ffcb3cd7b8772267be5/tests/cli_test.ts#L40-L44

Fresh island detection in tests

During the test run, inside the temporary directory there is deno.json and import-map with relative path.("@/": "./") The temporary directory is a short file name. The conditions for the bug to occur are now met. At this point, running the deno info command shows the following:

file:///C:/Users/RUNNER~1/AppData/Local/Temp/e77a1c79/main.ts (316B)
├─┬ .....
└─┬ file:///C:/Users/RUNNER~1/AppData/Local/Temp/e77a1c79/fresh.gen.ts (672B)
  ├── ....
  ├─┬ file:///C:/Users/RUNNER~1/AppData/Local/Temp/e77a1c79/routes/index.tsx (567B)
  │ ├── ....
  │ └─┬ file:///C:/Users/runneradmin/AppData/Local/Temp/e77a1c79/islands/Counter.tsx (550B) // !!!
  │   └─ ...
  └─┬ file:///C:/Users/RUNNER~1/AppData/Local/Temp/e77a1c79/islands/Counter.tsx (550B) // !!!
    ├── ...

The Counter component is imported twice from separate paths (runneradmin and RUNNER~1). Unfortunately, this has a negative impact on island detection. Inside render.ts, fresh detects the islands by comparing the Counter component references.

https://github.com/denoland/fresh/blob/7f3fd51cca75797ce9bf6ffcb3cd7b8772267be5/src/server/render.ts#L367-L373

Here we have a Counter component imported from fresh.gen.ts and a Counter component imported from routes/index.ts. Comparing these references yields false, so the island is not detected.

import Counter1 from "@/islands/Counter.tsx";
import Counter2 from "./islands/Counter.tsx";
Counter1 !== Counter2; // due to bugs.

Test failure

Failed to find island and the value of variable ENCOUNTERED_ISLANDS is 0. As a result, the page is considered static and no JS is delivered to the browser.

Telling the puppeteer to click doesn't work the counter. And the assertion at line 316 in cli_test.ts fails.

https://github.com/denoland/fresh/blob/7f3fd51cca75797ce9bf6ffcb3cd7b8772267be5/tests/cli_test.ts#L311-L316

After that, subsequent tests will also randomly fail because resource cleanup is not done.

How to respond

As for the deno.json bug, I'm not familiar with Rust so I'm not sure. Could it be the call to path.canonicalize on line 772 of cli/args/config_file.rs that is the problem?

ref: https://github.com/rust-lang/rust/issues/59117, https://github.com/rust-lang/rust/issues/76586,

https://github.com/denoland/deno/blob/25cbd97ab7ef1866e58238f1c28ec0d86062aee8/cli/args/config_file.rs#L772

Also, a possible workaround on the Fresh side, as described by https://github.com/actions/runner-images/issues/712#issuecomment-706524280, seems to be to override the TMP/TEMP environment variables in the GitHub Actions settings. Doing this will prevent short file names from being used for temporary directories, so CI will work.

# .github\workflows\ci.yml
      # Workaround for using temporary directory on Windows
      # https://github.com/actions/runner-images/issues/712
      - name: Set the TMP environment variable (Windows)
        if: startsWith(matrix.os, 'windows')
        run: echo "TMP=$env:USERPROFILE\AppData\Local\Temp" >> $env:GITHUB_ENV
      - name: Set the TEMP environment variable (Windows)
        if: startsWith(matrix.os, 'windows')
        run: echo "TEMP=$env:USERPROFILE\AppData\Local\Temp" >> $env:GITHUB_ENV

There are the result of debugging on https://github.com/ayame113/fresh/commits/win-failed-test branch.

[iuioiua]

Excellent troubleshooting, @ayame113! Very helpful. I've applied the workaround to CI.

Now, we're running into an issue caused by deno.json being in a parent directory instead of the standard root. I guess we could dynamically set the @/ import during the execution of the init script. Sound good, @lucacasonato?