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] PNPM readPackage hook does not resolve local unpublished packages #1766

Open
1 of 2 tasks
salieri opened this issue Mar 5, 2020 · 3 comments
Open
1 of 2 tasks
Labels
repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem

Comments

@salieri
Copy link

salieri commented Mar 5, 2020

Is this a feature or a bug?

  • Feature
  • Bug

Please describe the actual behavior.

It's not possible to refer to a local Rush package when fixing phantom dependencies using pnpmfile.js:readPackage()

If the issue is a bug, how can we reproduce it? Please provide detailed steps and include a GitHub branch if applicable. Your issue will get resolved faster if you can make it easy to investigate.

Modify pnpmfile.js:readPackage() so that it injects dependency to an NPM package managed by the Rush project, but which has not been published to NPM.

Example project: https://github.com/salieri/rush-local-dependency-issue

  1. Create two local packages and add them to rush.json
  2. Add an external dependency to one of the local packages
  3. Update common/config/rush/pnpmfile.js to inject a local package dependency into the external dependency
  4. rush update --full will fail, as PNPM attempts to pull the injected dependency from registry.npmjs.org

ERROR  404 Not Found: @rush-bug/package-a (via https://registry.npmjs.org/@rush-bug%2Fpackage-a)

This issue occurs for example when trying to use a custom react-scripts package with Storybook – @storybook/preset-create-react-app tries to import the custom react-scripts package from its own dependencies, where the locally hosted package does not reside.

What is the expected behavior?

Rush should resolve/link the injected local package.

Known workarounds

Publish the local package.

If this is a bug, please provide the tool version, Node.js version, and OS.

  • Tool: Rush Multi-Project Build Tool
  • Tool Version: 5.20.0
  • Node Version: 12.13.1
    • Is this an LTS version? Yes
    • Have you tested on an LTS version? Yes
  • OS: MacOS 10.15.3
@iclanton
Copy link
Member

This scenario isn't currently supported, but hopefully will be in the future.

The issue is twofold:

  • Rush creates "temp projects" for a repo's local projects for the purpose of package installation, which have different names from the project names in the repo. In your case, the temp project's name is @rush-temp/package-a. You can see this if you run rush update and look in the common/temp/projects/package-a/package.json file's "name" field. The code that generates those names is here.
  • When Rush links projects together and links dependencies to repo projects, it doesn't use the package manager. So even if you were to change the name you're using in pnpmfile.js to the temp project's name, PNPM would still query the registry for @rush-temp/package-a, and it wouldn't link your project in your repo back to lodash.

We've been discussing handing linking off to the package manager, and doing that would probably add support for this.

@apostolisms and @octogonz - This is another scenario that would be improved by handing linking off to the package manager.

@octogonz octogonz added the repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem label Mar 27, 2020
@octogonz
Copy link
Collaborator

It took some thinking, but I agree that this is a legitimate need (even though it seems weird heheh).

I agree that #1553 is the best solution for this problem.

It seemed like a workaround might be possible using something like this:

common/config/rush/pnpmfile.js

function readPackage(packageJson, context) {
  if (packageJson.name === 'lodash') {
    console.log('_________________________');
    context.log("Adding local dependency '@rush-bug/package-a' to 'lodash'");
    packageJson.dependencies['@rush-bug/package-a'] = 'file:../../packages/package-a/';
  }
  return packageJson;
}

However it runs into this PNPM error:

packages/supi/src/resolveDependencies.ts

  if (pkgResponse.body.isLocal) {
    const manifest = pkgResponse.body.manifest || await pkgResponse['fetchingRawManifest'] // tslint:disable-line:no-string-literal
    if (options.currentDepth > 0) {
      logger.warn({
        message: `Ignoring file dependency because it is not a root dependency ${wantedDependency}`,
        prefix: ctx.prefix,
      })
    } else {

@thelinuxlich
Copy link

Is there any update to this issue?

@iclanton iclanton moved this to Needs Investigation in Bug Triage Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repro confirmed The issue comments included repro instructions, and the maintainers reproduced the problem
Projects
Status: Needs Investigation
Development

No branches or pull requests

4 participants