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

fix: Unvirtualize importer paths in webpack and rspack #430

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

edemaine
Copy link
Contributor

I found that unplugin in webpack mode (and rspack mode) virtualized my resolved filenames (because I added a custom extension and the file didn't exist), but didn't unvirtualize the filenames when giving them back to me via the importer second argument to resolveId.

This simple PR fixes this issue by decoding virtualized filenames in importer, so that webpack and rspack behave like other systems. Includes a test with a simple mangling of filename (adding an extra .js to end of .js files).

Copy link

pkg-pr-new bot commented Oct 23, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/unjs/unplugin@430

commit: ea94e55

@edemaine edemaine force-pushed the unvirtualize-importer branch from f72854e to ea94e55 Compare October 23, 2024 21:33
@edemaine edemaine changed the title Unvirtualize importer paths in webpack and rspack fix: Unvirtualize importer paths in webpack and rspack Oct 23, 2024
@antfu antfu requested a review from sxzz October 24, 2024 16:21
Copy link
Member

@antfu antfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the high-level this makes sense to me. I guess ideally this should be a non-breaking fix?

/cc @ahabhgk can you also help to check the rspack part to see if it's expected? Thanks

@edemaine
Copy link
Contributor Author

I guess ideally this should be a non-breaking fix?

I think so... I can't imagine anyone doing something useful with the virtual importer filename. At least, the main use I know of for importer is to resolve relative ids relative to the importer, and without this change, you can't really do that (without painful workarounds). Given that the virtualization is done entirely within unplugin, I think this PR changes the behavior to be in line with expectation.

@ahabhgk
Copy link
Collaborator

ahabhgk commented Oct 25, 2024

Thanks, the rspack part looks good to me

@antfu antfu merged commit 52f0b79 into unjs:main Oct 28, 2024
12 checks passed
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

Successfully merging this pull request may close these issues.

3 participants