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

[rush] Rush install fails when there is a preferred version, possibly only with a peer depedency #1349

Closed
kelseyyoung opened this issue Jul 1, 2019 · 5 comments
Labels
bug Something isn't working as intended repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem

Comments

@kelseyyoung
Copy link

Using pnpm 3.5.3 and rush 5.10.0

If you add a library to preferredVersions that has a peer dependency, after an initial rush update any install or update will fail with an "Error: Invalid version"

I don't have an example pushed up to github, but locally I was using react-loadable (which has a peer of react) and so in the pnpm-lock looked like this:

  /react-loadable/[email protected]:
    dependencies:
      prop-types: 15.7.2
      react: 16.8.6
    dev: false
    peerDependencies:
      react: '*'
    resolution:
      integrity: sha512-C8Aui0ZpMd4KokxRdVAm2bQtI03k2RMRNzOB+IipV3yxFTSVICv7WoUr5L9ALB5BmKO1iHgZtWM8EvYG83otdg==

My preferredVersions has react-loadable here:

  "preferredVersions": {
    "react-loadable": "5.5.0"
  },

And both are included as dependencies in a package.json. The error I see is:

ERROR: Invalid Version: [email protected]

Removing react-loadable from preferredVersions removes the error, and so I think the issue stems from there

@rakeshpatnaik
Copy link

Can you please provide a repro repo to dig into this further?

@kelseyyoung
Copy link
Author

@rakeshpatnaik clone: https://github.com/kelseyyoung/RushInstallError and run 'rush install'

@octogonz octogonz added repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem bug Something isn't working as intended labels Jul 3, 2019
@octogonz
Copy link
Collaborator

octogonz commented Jul 3, 2019

Thanks for the repro!

This issue is caused by the file format changes in PNPM 3.

If I regenerate your shrinkwrap file using PNPM 2.x (latest), I get this:

dependencies:
  '@rush-temp/test-app': 'file:projects/test-app.tgz'
  react: 16.8.6
  react-dom: 16.8.6
  react-loadable: 5.5.0
packages:
  . . .
  /react-loadable/5.5.0:
    dependencies:
      prop-types: 15.7.2
    dev: false
    peerDependencies:
      react: '*'
    resolution:
      integrity: sha512-C8Aui0ZpMd4KokxRdVAm2bQtI03k2RMRNzOB+IipV3yxFTSVICv7WoUr5L9ALB5BmKO1iHgZtWM8EvYG83otdg==
  /react-loadable/5.5.0/[email protected]:
    dependencies:
      prop-types: 15.7.2
      react: 16.8.6

...whereas with PNPM 3 it looks like this:

dependencies:
  '@rush-temp/test-app': 'file:projects/test-app.tgz'
  react: 16.8.6
  react-dom: [email protected]
  react-loadable: [email protected]
packages:
  . . .
  /react-loadable/[email protected]:
    dependencies:
      prop-types: 15.7.2
      react: 16.8.6

Thus the version 5.5.0 has been replaced by a cross reference to [email protected] which specifies the installation of 5.5.0 whose peer is satisfied using React 16.8.6.

Rush's installation logic validates the 5.5.0 preferred version like this:

https://github.com/microsoft/web-build-tools/blob/cf40afdbff7c793b14242654ffc8a951a8cc6dff/apps/rush-lib/src/logic/InstallManager.ts#L504-L511

That causes the bad token [email protected] to end up as dependencyVersion here, which eventually gets passed to semver.satisfies(), which can't parse it:
https://github.com/microsoft/web-build-tools/blob/49d94443ad43d12dbb8bb97f1b00ec8dcb9df5b6/apps/rush-lib/src/logic/pnpm/PnpmShrinkwrapFile.ts#L218-L222

Seems like that code was already handling the /react-loadable/5.5.0/[email protected] form. Maybe it simply needs to be updated to handle [email protected] as well?

@nickpape-msft

@kelseyyoung
Copy link
Author

Any update on this? This is preventing us from upgrading, and seems like a very common issue...

@apostolisms
Copy link
Contributor

@kelseyyoung this should be fixed now in version 5.10.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem
Projects
Archived in project
Development

No branches or pull requests

4 participants