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

Resolve fails on MacOS and Windows, works on linux #22

Closed
dcarniel-ipepper opened this issue Jan 30, 2024 · 7 comments
Closed

Resolve fails on MacOS and Windows, works on linux #22

dcarniel-ipepper opened this issue Jan 30, 2024 · 7 comments

Comments

@dcarniel-ipepper
Copy link

dcarniel-ipepper commented Jan 30, 2024

We have a SvelteKit project (which uses your library in version 4.0.0) which runs fine on my Debian VM but fails to resolve a dependency both on a Mac and a Windows machine when using Yarn pnp linker (it works everywhere with Yarn node-modules liker though).

After clearing out any other reason (fresh branch checkout on all machines, no changes...) I had to trace the code, and I ended up looking into SvelteKit code and figured the following line didn't behave the same between platforms:

const resolved = await imr.resolve(dependency, pathToFileURL(process.cwd() + '/dummy.js'));

On my linux VM I get a path, on the other machine an exception. On this line imr is being imported from your library with: import * as imr from 'import-meta-resolve';.

Since I use Node 20, I tried changing the line to:

const resolved = import.meta.resolve(dependency, pathToFileURL(process.cwd() + '/dummy.js'));

And everything works on all machines.

For reference, this is the location of the call in the SvelteKit project: https://github.com/sveltejs/kit/blob/a6333a5d0833781c6572098a6240976e84fb5150/packages/kit/src/exports/vite/index.js#L134

I must admit I don't have a clue as to what causes the issue, in particular we have no space in the paths and they are reasonable in length (48 to 65 characters depending on platform).

Don't hesitate to let me know if there a place where I could look for more details to help solve this.

@wooorm
Copy link
Owner

wooorm commented Jan 30, 2024

Can you reduce this. What values are being passed. Which things exist on disk?

I am on mac, it works.

process.cwd() + '/dummy.js' seems quite bad on your side. / doesn’t work on Windows. I’d also guess process.cwd() to be different across OSes. process.cwd() typically ends in path.sep already. path.join(process.cwd(), 'dummy.js') is how you combine paths in Node.

@ts-expect-error the types are wrong

The types here are not wrong? Your await is wrong.

@wooorm
Copy link
Owner

wooorm commented Jan 30, 2024

Yarn pnp linker

This sounds quite likely to be the problem too. The algorithm here is based on how Node/npm work. Yarn intentionally does things differently. Perhaps you can see things posted already in its issue tracker

@dcarniel-ipepper
Copy link
Author

Can you reduce this. What values are being passed. Which things exist on disk?

I am on mac, it works.

process.cwd() + '/dummy.js' seems quite bad on your side. / doesn’t work on Windows. I’d also guess process.cwd() to be different across OSes. process.cwd() typically ends in path.sep already. path.join(process.cwd(), 'dummy.js') is how you combine paths in Node.

My first thought was indeed around an issue with cwd, I first changed the code to use path.join to avoid any such issue, but it didn't change a thing.

Regarding the "dummy.js", my understanding is that they (it's not my code) want to resolve a peer dependency, so they concatenate the current directory with a fictitious filename so the resolver thinks it is working for an actual file.

In the end it passes a path like /home/user/project/dummy.js and @sveltejs/kit. The only point I could see being a problem is that the "file" used as reference is not in the "src" folder of the project.

@dcarniel-ipepper
Copy link
Author

Yarn pnp linker

This sounds quite likely to be the problem too. The algorithm here is based on how Node/npm work. Yarn intentionally does things differently. Perhaps you can see things posted already in its issue tracker

I know it behave differently, though it works on my Linux VM, plus when I change the code to use import.meta.resolve from node it works like a charm on all machines.

@wooorm
Copy link
Owner

wooorm commented Jan 30, 2024

(it's not my code)

It’s not my code either 😉.
In my experience, based on tests, this project works perfectly in macOS, linux, and windows: https://github.com/wooorm/import-meta-resolve/actions/runs/6734940885.
Now it’s on you to create a reduced test case showing it doesn’t!

Showing which results are generated for macos/windows/linux, and which files exist in those 3 cases, will help.

Regarding the "dummy.js"

It’s because .cwd() points to a folder.
resolve works from files.
Folders are also files.
But there are differences in resolving from a, a/, and a/b.js.

plus when I change the code to use import.meta.resolve

I believe there are specific things that yarn does for pnp to hook into import.meta.resolve. If you search for “yarn pnp import.meta.resolve” you’ll find sveltejs/kit#11578 (comment) for example

@dcarniel-ipepper
Copy link
Author

Okay I thought changing this specific call was a clear enough indicator that the code performed differently on different environments, if you don't feel like looking into it, I'll see if I have some time to create a more "isolated" reproducer.

@wooorm
Copy link
Owner

wooorm commented Jan 30, 2024

See also #10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants