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

A file might be imported multiple times on Windows #151

Open
ah-dc opened this issue Jul 19, 2023 · 3 comments
Open

A file might be imported multiple times on Windows #151

ah-dc opened this issue Jul 19, 2023 · 3 comments
Labels

Comments

@ah-dc
Copy link
Contributor

ah-dc commented Jul 19, 2023

Environment

Windows

Reproduction

I created one on stackblitz but it is on Linux, so please download it and run it on Windows.
I met this bug when I was using stub mode of unbuild, so this reproduction is based on that.

https://stackblitz.com/edit/stackblitz-starters-m3lfbv1

Describe the bug

Windows support \ and / as path separator, so a file might be imported more than once with different path on Windows, for example C:\foo.ts and C:/foo.ts. It causes problems when this file has side effects.

Additional context

No response

Logs

No response

@ah-dc
Copy link
Contributor Author

ah-dc commented Jul 19, 2023

I have to say sorry that this issue is completely misleading... After some investigation I found the true issue.

First of all, the multiple paths only affects the cache, jiti uses filename to do hash so different paths will lead to different transpiled files, but it won't cause a file to be imported more than once, just a minor performance issue.

The real cause is

jiti/src/jiti.ts

Lines 154 to 156 in 79eeafb

const _additionalExts = [...(opts.extensions as string[])].filter(
(ext) => ext !== ".js"
);

jiti/src/jiti.ts

Lines 173 to 176 in 79eeafb

resolved = resolvePathSync(id, {
url: _url,
conditions,
});

I think we should pass _additionalExts into mlly resolve options

@ah-dc ah-dc changed the title A file might be imported multiple times on Windows ESM resolution doesn't support custom extra extensions Jul 19, 2023
@ah-dc
Copy link
Contributor Author

ah-dc commented Jul 19, 2023

But I don't know why it works on linux, maybe it got successfully resolved via another branch

@ah-dc
Copy link
Contributor Author

ah-dc commented Jul 19, 2023

Update: It is still possible to cause duplicated imports by using different paths on Windows.

Now I think we should split this issue into 2.

@ah-dc ah-dc changed the title ESM resolution doesn't support custom extra extensions A file might be imported multiple times on Windows Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants