Skip to content

feat: ✨ allow file type to be called with unslashed dir path #4

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

Merged
merged 1 commit into from
Apr 20, 2025

Conversation

stormslowly
Copy link
Contributor

before

zip.file_type("node_modules") returns Error NotFound
zip.file_type("node_modules/") returns Directory

after

zip.file_type("node_modules") returns Directory
zip.file_type("node_modules/") returns Directory

background

try to resolve ./devtools from preact-xxxxxx.zip/node_modules/preact

zip.file_type("node_modules/preact/devtools") returns NotFound, results in rspack-resolve failed.

@stormslowly
Copy link
Contributor Author

@arcanis could you plz have a look

@stormslowly
Copy link
Contributor Author

in my opinion when the path is inside a zip file, resolver can not normalize that correctly before call zip apis.

For example, xxxxx.zip/node_modules/some-npm/README resolver don't know it is a file or dir, so it can not normalize that then to call file_type api ; or just try both xxxxx.zip/node_modules/some-npm/README and xxxxx.zip/node_modules/some-npm/README/

@arcanis
Copy link
Member

arcanis commented Feb 17, 2025

While I understand the pickle, my problem is that I don't understand (haven't been able to dig too deep yet) why it seems to work in rspack but not in your case.

For instance if we do require('typanion/lib'), it works fine in rspack even without this trailing / - and we don't seem to do anything magic. So I wonder if there's something I forgot that would make this a non-issue.

@stormslowly
Copy link
Contributor Author

@arcanis we got an new issue due to the Zip#file_type API.
plz have a look, web-infra-dev/rspack#9967 (comment)

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.

2 participants