Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent SSR require/import paths on Windows #6080

Closed
7 tasks done
Brendan-csel opened this issue Dec 12, 2021 · 16 comments
Closed
7 tasks done

Inconsistent SSR require/import paths on Windows #6080

Brendan-csel opened this issue Dec 12, 2021 · 16 comments

Comments

@Brendan-csel
Copy link

Brendan-csel commented Dec 12, 2021

Describe the bug

The following entries appear in require.cache on Windows...

[
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\web\\dist\\server.cjs',
  'C:/DevTemp/my-app/node_modules/solid-js/dist/server.cjs',
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\dist\\server.cjs'
]

Note the 2nd and 3rd paths above refer to the same file.
Whereas on Linux in the same project we see...

[
  '/home/brendan/devtemp/my-app/node_modules/solid-js/web/dist/server.cjs',
  '/home/brendan/devtemp/my-app/node_modules/solid-js/dist/server.cjs'
]

The second entry results from a require() call within the preceding .cjs file. Whereas the others result from Vite calling its dynamicImport() function.

On Windows the duplication of the second and third import above results in errors in this particular library as it is expected to run once.

These were observed by calling console.log(Object.keys(require.cache).filter(k => k.includes("solid-js"))) in strategic locations on both OSs.

Reproduction

mkdir my-app
cd my-app
npm init solid@next  (choose Typescript)
npm install
npm run dev

Browse http://localhost:3000/ and observe an error (on Windows only).

Add this console log somewhere: console.log(Object.keys(require.cache).filter(k => k.includes("solid-js")))
eg. line 67385 of node_modules\vite\dist\node\chunks\dep-3daf770c.js

    try {
        const mod = await dynamicImport(url);
        console.log(Object.keys(require.cache).filter(k => k.includes("solid-js")))
        return proxyESM(mod);
    }
    finally {
        unhookNodeResolve();
    }

Observe the above mentioned require.cache entries differ from Windows to Linux.

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (12) x64 Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
    Memory: 5.52 GB / 15.72 GB
  Binaries:
    Node: 16.13.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.5 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 8.3.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (96.0.1054.43)
    Internet Explorer: 11.0.22000.120
  npmPackages:
    vite: ^2.7.1 => 2.7.1

Used Package Manager

npm

Logs

No response

Validations

@Brendan-csel
Copy link
Author

Brendan-csel commented Dec 12, 2021

The last version that seemed to work on Windows was Vite 2.7.0-beta.3 which returns...

[
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\web\\dist\\server.cjs',
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\dist\\server.cjs'
]

2.7.0-beta.4 and 2.7.0-beta.5 have some other possibly unrelated errors.

2.7.0-beta.6 onwards exhibits the behaviour described in this issue...

[
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\web\\dist\\server.cjs',
  'C:/DevTemp/my-app/node_modules/solid-js/dist/server.cjs',
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\dist\\server.cjs'
]

@Brendan-csel Brendan-csel changed the title Inconsistent SSR import paths on WIndows Inconsistent SSR import paths on Windows Dec 12, 2021
@Brendan-csel Brendan-csel changed the title Inconsistent SSR import paths on Windows Inconsistent SSR require/import paths on Windows Dec 12, 2021
@Brendan-csel
Copy link
Author

Just tested recently released 2.7.2 but same issue.

Best place to console.log when testing with 2.7.2 is in dep-7817f5b4.js line 67483:

    try {
        const mod = await dynamicImport(url);
        //add log here to see issue
        console.log(Object.keys(require.cache).filter(k => k.includes("solid-js")))
        return proxyESM(mod);
    }

Result in require.cache on Windows only:

[
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\web\\dist\\server.cjs',
  'C:/DevTemp/my-app/node_modules/solid-js/dist/server.cjs',
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\dist\\server.cjs'
]

@Brendan-csel
Copy link
Author

Tested latest 2.7.3 which has same issue.

Place to log is line 67508 in node_modules\vite\dist\node\chunks\dep-5496817b.js

@Brendan-csel
Copy link
Author

Tested latest 2.7.5 which has same issue.

Place to log is line 67835 in node_modules\vite\dist\node\chunks\dep-bc5f477e.js

@Brendan-csel
Copy link
Author

Tested latest 2.7.6 which has same issue.

Place to log is line 67835 in node_modules\vite\dist\node\chunks\dep-fcec4469.js

@Brendan-csel
Copy link
Author

Possible hack/solution?
In ssrModuleLoader.ts wrapping the last line shown below in normalizePath() from ../utils stops the solid-js module being imported twice.

 // When an ESM module imports an ESM dependency, this hook is *not* used.
  const unhookNodeResolve = hookNodeResolve(
    (nodeResolve) => (id, parent, isMain, options) => {
      // Fix #5709, use require to resolve files with the '.node' file extension.
      // See detail, https://nodejs.org/api/addons.html#addons_loading_addons_using_require
      if (id[0] === '.' || isBuiltin(id) || id.endsWith('.node')) {
        return nodeResolve(id, parent, isMain, options)
      }
      if (parent) {
        return viteResolve(id, parent.id)
      }
      // Importing a CJS module from an ESM module. In this case, the import
      // specifier is already an absolute path, so this is a no-op.
      // Options like `resolve.dedupe` and `mode` are not respected.
      return normalizePath(id) // ******************* HACK adding normalizePath  (need to import this from utils.ts) *********************
    }
  )

I'm not sure quite why though as require.cache looks worse when this hack is in place...

[
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\web\\dist\\server.cjs',
  'C:/DevTemp/my-app/node_modules/solid-js/web/dist/server.cjs',
  'C:/DevTemp/my-app/node_modules/solid-js/dist/server.cjs',
  'C:\\DevTemp\\my-app\\node_modules\\solid-js\\dist\\server.cjs'
]

Maybe this is at least a clue for someone who understands this stuff??

@Brendan-csel
Copy link
Author

VERY INTERESTING...

It seems when import is called with POSIX path like C:/DevTemp/... it only adds 1 entry in require.cache.

BUT, if import is called with a non-POSIX path like C:\DevTemp\... it adds 2 entries into require.cache (the non-POSIX version and POSIX version) both resolving to the same module.

These seem to be the steps occurring with the problem example:

  1. solid-start\runtime\entries\nodeStream.tsx imports solid-js/web which resolves to a non-POSIX path C:\...\web\dist\server.cjs resulting in 2 entries in require.cache for that module.
  2. The above module calls require('solid-js') which resolves to a POSIX path and therefore only 1 entry is added to require.cache
    ...later....
  3. solid-meta/dist/index.jsx imports solid-js directly which resolves to a non-POSIX path C:\...solid-js\dist\server.cjs. That is not found in require.cache (due to previous step only adding 1 entry) so the module loads/runs again..

So the paths resolved from the .jsx import is a different format (non-POSIX) to the paths resolved from .cjs require (POSIX).

SUMMARY
It seems that in older versions Vite (on Windows) used to always resolve to non-POSIX paths resulting in both options being seeded in require.cache. That has changed since about 2.7.0-beta.4? and is now resolving to POSIX paths sometimes - meaning require.cache is not seeded with both options. In turn, when the non-POSIX version is requested later the module is loaded again.

Hopefully I've got that all straight and it makes sense to someone who knows a lot more about this stuff.

@Brendan-csel
Copy link
Author

Brendan-csel commented Dec 28, 2021

Tested latest 2.7.7 which has same issue.

Place to log is line 67846 in node_modules\vite\dist\node\chunks\dep-4a9cff06.js

Although I think the most useful comment is here

@Brendan-csel
Copy link
Author

Tested latest 2.7.9 which has same issue.

Place to log is line 60152 in node_modules\vite\dist\node\chunks\dep-4b9dfa16.js

Although I think the most useful comment is here

@Niputi
Copy link
Contributor

Niputi commented Dec 30, 2021

you are welcome to create pull request

@Brendan-csel
Copy link
Author

you are welcome to create pull request

Thanks - but I wouldn't know where to start. I'm just blindly trying to provide as much info as I can muster.

@ryansolid
Copy link

Thanks @Brendan-csel for looking into this. I don't have the means to debug this, but seems to break every Solid SSR project on Windows. Which makes this a high priority for me.

I will see if I can do anything to help.

@Brendan-csel
Copy link
Author

Tested latest 2.7.10 which has same issue.

Place to log is line 60153 in node_modules\vite\dist\node\chunks\dep-76613303.js

Although I think the most useful comment is here

@Brendan-csel
Copy link
Author

Tested latest v2.8.0-beta.1 which has same issue.

Place to log is line 55594 in node_modules\vite\dist\node\chunks\dep-e4dc9ea2.js

Although I think the most useful comment is here

@Brendan-csel
Copy link
Author

This issue seems to be resolved in 2.7.13. Thanks to all involved!

@patak-dev
Copy link
Member

Thank you @Brendan-csel for all the testing in the issue!

@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants