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

resolveId expanding mapped network drives in file names #525

Closed
stephenlrandall opened this issue Dec 7, 2022 · 5 comments
Closed

resolveId expanding mapped network drives in file names #525

stephenlrandall opened this issue Dec 7, 2022 · 5 comments
Labels
bug Something isn't working edge-case

Comments

@stephenlrandall
Copy link

stephenlrandall commented Dec 7, 2022

Describe the bug

There is a discrepancy in resolved path names between vite:resolve and vite-plugin-svelte. The latter's plugin function resolveId is using findDepPkgJsonPath from "vitefu", which expands mapped drive names. Looking at a particular dependency:

dep: svelte-select
parent: Z:/<repo path>/src/components/elements/Select.svelte?id=0
dep data path: \\<network drive>\<repo path>\node_modules\svelte-select\package.json

This is returned to Vite as the resolved object:

{ id: '/<network drive>/<repo path>/node_modules/svelte-select/index.js' }

By contrast, a non-package.svelte dependency (for example, math.js) is skipped and handled by vite:resolve instead:

{ id: 'Z:/<repo path>/node_modules/mathjs/lib/esm/index.js' }

This discrepancy leads to the "file not found" error shown in the log section below.


I wasn't sure if this issue would better fit here or in vitefu. But this comment referenced using Vite's resolve, which -- if that's the same as vite:resolve -- would fix this issue.

Reproduction URL

not reproducible with a repo alone

Reproduction

On a mapped network drive, set up a new SvelteKit project with at least one dependency that uses the svelte package field.

Logs

node:fs:591
  handleErrorFromBinding(ctx);
  ^

Error: ENOENT: no such file or directory, open '/<network drive>/<repo path>/node_modules/svelte-select/index.js'
    at Object.openSync (node:fs:591:3)
    at Object.readFileSync (node:fs:459:35)
    at extractExportsData (file:///Z:/<repo path>/node_modules/vite/dist/node/chunks/dep-5605cfa4.js:42115:31)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
  errno: -4058,
  syscall: 'open',
  code: 'ENOENT',
  path: '/<network drive>/<repo path>/node_modules/svelte-select/index.js'
}

System Info

System:
    OS: Windows 10
    CPU: (6) x64 Intel(R) Xeon(R) Gold 6226R CPU @ 2.90GHz
    Memory: 3.91 GB / 12.00 GB
  Binaries:
    Node: 18.2.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.9.0 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    @sveltejs/adapter-static: ^1.0.0-next.48 => 1.0.0-next.48
    @sveltejs/kit: 1.0.0-next.572 => 1.0.0-next.572
    svelte: ^3.53.1 => 3.53.1
    vite: 3.2.5 => 3.2.5
@stephenlrandall stephenlrandall added bug Something isn't working triage Awaiting triage by a project member labels Dec 7, 2022
@dominikg
Copy link
Member

dominikg commented Dec 7, 2022

thank you for this very detailed report.

We are in the process of phasing out support for resolving via "svelte" field, the PR you linked is the first step to that but it'll take months at best for it to be completed.

In the meantime vite-plugin-svelte itself can't do much to remedy this situation, maybe vitefu could be improved to return the expected path in this situation, but i recommend not using network drives for development in general. This can cause problems with file change detection, increases the load on your system and generally introduces another point of failure in an already pretty complex system.

@dominikg dominikg added edge-case and removed triage Awaiting triage by a project member labels Dec 7, 2022
@stephenlrandall
Copy link
Author

I appreciate the response — I wish not using network drives was an option, but unfortunately it’s a requirement in this case.

I can work around this for now by forcing this plugin’s resolveId to always return undefined so that it gets skipped; do you know if that has adverse side-effects that I should be aware of?

maybe vitefu could be improved to return the expected path in this situation

@bluwy I believe you’re the right person to ping for this; would updating the vitefu path resolver be possible in a shorter time frame? I can try to put together a PR that addresses my use case but I don’t have the context of everywhere that function is used and what effects a change like this would have.

@bluwy
Copy link
Member

bluwy commented Dec 7, 2022

@stephenlrandall if you can send a PR to vitefu with a fix, that'll be great! it would likely need a fix here. I'm not sure how a leading backslash happens but if the fix can be done for Windows specifically it should be good.

@stephenlrandall
Copy link
Author

stephenlrandall commented Dec 16, 2022

@bluwy Found something I didn't expect. If you add some print statements to the function you linked:

console.log(`pkg: ${pkg}`);
try {
  await fs.access(pkg);

  // -------------------------------------------------
  console.log(`sync: ${fsSync.realpathSync(pkg)}`);
  const asyncRes = await fs.realpath(pkg);
  console.log(`async: ${asyncRes}`);
  // -------------------------------------------------

  return await fs.realpath(pkg);
}

what you get is:

pkg: Z:\<repo path>\node_modules\svelte-select\package.json
sync: Z:\<repo path>\node_modules\svelte-select\package.json
async: \\<network drive>\<repo path>\node_modules\svelte-select\package.json

Vite, in getRealPath within vite:resolve, uses realpathSync from node:fs. vitefu uses realpath from node:fs/promises. As to why these functions have different behavior, there's some discussion in this issue. (The relevant functions are here, for reference.)

As a fix (and to at least be consistent with Vite proper), would it be possible to simply replace the node:fs/promises version with the node:fs version? I've opened a PR in vitefu with that change.

@bluwy
Copy link
Member

bluwy commented Dec 17, 2022

Published vitefu v0.2.4 with the fix. When freshly installing vite-plugin-svelte, it should get the latest version of it now. Closing as fixed. Thanks again for debugging this!

@bluwy bluwy closed this as completed Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working edge-case
Projects
None yet
Development

No branches or pull requests

3 participants