[rush] Add support for package aliases ("@npm:" in version specifier)

[Kevin Grandon (KevinGrandon)]

Hello, would love some guidance, or if you want to take this over and land it I'd be happy with that, but I'm very happy to continue contributing to this. This PR is currently working with my test case , but I have been unable to figure out how to write a properly functioning test case for this.

This is an attempt to add support for finding packages when their semver range starts with npm:, which is a useful mechanism for aliasing or forking packages. See #1279 for more details.

Refs #1279

[Ian Clanton-Thuon (iclanton)]

Interesting. I've never seen this feature before. This is a feature of Yarn? Do you have a repro repo that we can test this against?

[Kevin Grandon (KevinGrandon)]

Interesting. I've never seen this feature before. This is a feature of Yarn? Do you have a repro repo that we can test this against?

Hello, thanks for chiming in. Yes, this is a feature of Yarn. I've created a test repository here: https://github.com/KevinGrandon/bug-rush-yarn-npm-resolution

You can clone it and run rush install to reproduce the issue. If you cd example-fusion-app and run yarn, you will see that everything works.

See Yarn aliasing for more info: https://yarnpkg.com/en/docs/cli/add#toc-yarn-add-alias

[Kevin Grandon (KevinGrandon)]

Latest changes to npm/NpmLinkManager.ts get this working in my example repo. Tests still need help.

[Kevin Grandon (KevinGrandon)]

Tried with the following ranges and this seems to work ok:

// Existant package, works
"fusion-core": "npm:fusion-core@1.0.0",
// Existant namespaced package, works
"fusion-core": "npm:@kevingorg/fusion-core",
// Existant namespaced package with range, works
"fusion-core": "npm:@kevingorg/fusion-core@0.0.1",
// Non-existant namespaced package with range, errors as expected
"fusion-core": "npm:@kevingorg/fusion-core@1000.0.0",

[Pete Gonzalez (octogonz)]

In #1279 (comment) Zoltan said:

This is supported by pnpm, npm and jspm as well. (Actually, it was first introduced in jspm)

I wonder if we should implement this for PNPM as well (but maybe in a separate PR?).

[Pete Gonzalez (octogonz)]

This PR is awaiting the design discussion in #1279

I'm starting to believe it's an important feature, so if I get some time I may pitch in with the implementation.

[Josh Wedekind (halfnibble)]

Kevin Grandon (@KevinGrandon) Do you have the information needed to proceed with this WIP, or is the design still unclear? (Or do you want someone else to take over this PR?) Thanks.

[Pete Gonzalez (octogonz)]

Josh Wedekind (@halfnibble) I believe we arrived at a design.

Since all the package managers support this feature, it seems useful for Rush to support it as well. If Kevin doesn't have time, perhaps someone else could help out.

The main design changes that I had requested were:

• Make the command line like rush add --package @acme/foo@~1.2.3 --alias @acme/my-foo; the NPM style is rush add --package @acme/my-foo@npm:@acme/foo@~1.2.3 which is too confusing

• Change the parser as described in [rush] Add support for package aliases ("@npm:" in version specifier) #1279 (comment). The current PR uses versionRange.indexOf('npm:') === 0 which will not work correctly for certain variants. (The important token is @ not npm:.)

[Kevin Grandon (KevinGrandon)]

Since all the package managers support this feature, it seems useful for Rush to support it as well. If Kevin doesn't have time, perhaps someone else could help out.

Since all the package managers support this feature, it seems useful for Rush to support it as well. If Kevin doesn't have time, perhaps someone else could help out.

Thanks for the updates here, I am short on time and not sure I am the right person to implement the parser. Please feel free to steal the code diff here or close this PR. Apologies I wasn't able to push this over the finish line.

[Pete Gonzalez (octogonz)]

This PR is now code complete and seems to work with both PNPM and Yarn. The code review may be a bit tricky since I had to do a fair amount of refactoring.

Overall I believe this change should also improve some edge cases where rush update may have sometimes incorrectly analyzed the shrinkwrap file for PNPM.

Ian Clanton-Thuon (@iclanton)

[Pete Gonzalez (octogonz)]

Ian Clanton-Thuon (@iclanton) let's try to get this merged.