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.

Sign up for GitHub

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

Jump to bottom

src: do not treat all paths ending with node_modules as such #51269

Merged
merged 1 commit into from
Dec 25, 2023
Merged

src: do not treat all paths ending with node_modules as such #51269

merged 1 commit into from
Dec 25, 2023

Conversation

targos
Copy link
Member

@targos targos commented Dec 23, 2023

Fixes: #51245

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Dec 23, 2023
aduh95
aduh95 approved these changes Dec 23, 2023
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

LGTM. Is / going to work on Windows though?

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Dec 23, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 23, 2023
@targos
Copy link
Member Author

targos commented Dec 23, 2023

I suppose, yes. It's compared with a parsed URL.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56475/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 23, 2023 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/56479/ 💛

anonrig
anonrig approved these changes Dec 23, 2023
View reviewed changes
Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

Nice catch. Thank you for fixing this!

@jasnell jasnell added commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Dec 23, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 25, 2023
@nodejs-github-bot nodejs-github-bot merged commit c64e603 into nodejs:main Dec 25, 2023
63 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c64e603

@ljharb
Copy link
Member

ljharb commented Dec 25, 2023

cc @nodejs/loaders

@targos targos deleted the fix-51245 branch December 25, 2023 20:02
RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
@targos @RafaelGSS
src: do not treat all paths ending with node_modules as such
864ecb0 
Fixes: #51245
PR-URL: #51269
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
Merged
@marco-ippolito marco-ippolito added the backport-blocked-v20.x PRs that should land on the v20.x-staging branch but are blocked by another PR's pending backport. label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@anonrig anonrig anonrig approved these changes

@lpinca lpinca lpinca approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v20.x PRs that should land on the v20.x-staging branch but are blocked by another PR's pending backport. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ESM loader cannot detect module system properly in folder whose name ends with node_modules
8 participants
@targos @nodejs-github-bot @ljharb @jasnell @lpinca @anonrig @aduh95 @marco-ippolito