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

Relative paths in noparse option #2080

Merged
merged 5 commits into from
Oct 3, 2024

Conversation

goldim
Copy link
Contributor

@goldim goldim commented Aug 24, 2024

Browserify command with relative paths via --noparse option doesn't work. If I pass full paths in the option everything works as expected. Digging the problem I noticed that noparse array are passed into module-deps class via constructor. And in this package there is no processing the array like in browserify package (basedir+cwd+file name in noparse array).

@mikixing

This comment was marked as spam.

@ljharb
Copy link
Member

ljharb commented Aug 24, 2024

This will need a regression test.

@goldim
Copy link
Contributor Author

goldim commented Aug 26, 2024

@ljharb Hello, you mean new tests/test in which I have to check passing relative paths for noParse argument, don't you? Maybe I've hurried creating this PR. I think an issue first would be good idea. What do you think?
Also I've found this issue in which the author complained about not working command browserify and he passed relative paths. In comments someone told him that paths have to be absolute.
But nowhere is said that paths have to be absolute.

@ljharb
Copy link
Member

ljharb commented Aug 26, 2024

I mean a test that would fail without the changes in this PR.

@goldim
Copy link
Contributor Author

goldim commented Aug 26, 2024

@ljharb Done.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Assuming that the test fails without your fix, and passes with it, this LGTM modulo nits

test/noparse.js Outdated Show resolved Hide resolved
test/noparse.js Outdated Show resolved Hide resolved
test/noparse.js Outdated Show resolved Hide resolved
goldim and others added 3 commits August 30, 2024 10:45
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
Co-authored-by: Jordan Harband <ljharb@gmail.com>
@goto-bus-stop goto-bus-stop merged commit c8f43c7 into browserify:master Oct 3, 2024
1 check failed
@mikixing

This comment was marked as spam.

@goto-bus-stop
Copy link
Member

Released in 📦 17.0.1. Thank you!

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

Successfully merging this pull request may close these issues.

4 participants