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: support deeply nested pnpm virtual store node_modules paths in resolveAndRunNgcc #1742

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

gregmagolan
Copy link
Contributor

@gregmagolan gregmagolan commented Aug 10, 2022

Uses indexOf instead of lastIndexOf in resolveAndRunNgcc.

This resolves the case where a user is using pnpm and ngcc is resolved through symlinks to a deeply nested virtual store node_modules location:

some/user/path/node_modules/.pnpm/@angular/compiler-cli@1.2.3/node_modules/@angular/compiler-cli/...

lastIndexOf would resolve to some/user/path/node_modules/.pnpm/@angular/compiler-cli@1.2.3 which is incorrect. indexOf gets you to the desired some/user/path

Copy link
Collaborator

@atscott atscott left a comment

Choose a reason for hiding this comment

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

Sorry to be a stickler, but you you update the commit message to describe what's fixed and why it was broken rather than the "what" of the code change? This will help anyone going through the commits know why this change was made (otherwise, there's no context other than our conversation on slack)

@gregmagolan gregmagolan force-pushed the index_of branch 2 times, most recently from f9e5e3a to 6b11195 Compare August 10, 2022 20:02
@gregmagolan gregmagolan changed the title fix: get indexOf node_modules instead of lastIndexOf in resolveAndRunNgcc fix: support deeply nested pnpm virtual node_modules paths in resolveAndRunNgcc Aug 10, 2022
@gregmagolan gregmagolan changed the title fix: support deeply nested pnpm virtual node_modules paths in resolveAndRunNgcc fix: support deeply nested pnpm virtual store node_modules paths in resolveAndRunNgcc Aug 10, 2022
…esolveAndRunNgcc

Uses indexOf instead of lastIndexOf in resolveAndRunNgcc.

This resolves the case where a user is using pnpm and ngcc is resolved through symlinks to a deeply nested virtual store node_modules location:

`some/user/path/node_modules/.pnpm/@angular/compiler-cli@1.2.3/node_modules/@angular/compiler-cli/...`

lastIndexOf would resolve to `some/user/path/node_modules/.pnpm/@angular/compiler-cli@1.2.3` which is incorrect. indexOf gets you to the desired `some/user/path`
@atscott atscott added target: patch This PR is targeted for the next patch release action: merge Ready to merge labels Aug 10, 2022
@atscott atscott merged commit 511218f into angular:main Aug 10, 2022
atscott pushed a commit that referenced this pull request Aug 10, 2022
…esolveAndRunNgcc (#1742)

Uses indexOf instead of lastIndexOf in resolveAndRunNgcc.

This resolves the case where a user is using pnpm and ngcc is resolved through symlinks to a deeply nested virtual store node_modules location:

`some/user/path/node_modules/.pnpm/@angular/compiler-cli@1.2.3/node_modules/@angular/compiler-cli/...`

lastIndexOf would resolve to `some/user/path/node_modules/.pnpm/@angular/compiler-cli@1.2.3` which is incorrect. indexOf gets you to the desired `some/user/path`

(cherry picked from commit 511218f)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge Ready to merge target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants