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

[BUG] npm install in a workspace project ignores the workspace #2546

Closed
jakobrosenberg opened this issue Jan 26, 2021 · 25 comments
Closed

[BUG] npm install in a workspace project ignores the workspace #2546

jakobrosenberg opened this issue Jan 26, 2021 · 25 comments
Labels
Awaiting Information further information is requested Enhancement new feature or improvement Needs Discussion is pending a discussion Release 7.x work is associated with a specific npm 7 release

Comments

@jakobrosenberg
Copy link

Current Behavior:

If I have

myworkspace/
  project1/
    package.json
  project2/         
    package.json    // "dependencies": { "project1": "^1.0.0" }
  package.json      // "workspaces": [ "*" ]

👍cd workspace && npm install links local project1 to project2 correctly.
👎cd workspace/project2 && npm install doesn't link local project1 package.

Expected Behavior:

npm install within a workspace should do the same whether it's in the workspace root or a workspace package.

Steps To Reproduce:

  1. Create a root workspace
  2. Init two nested projects
  3. npm install [1st project] from 2nd project

Environment:

  • OS: Windows 10
  • Node: 15.6.0
  • npm: 7.4.0
@jakobrosenberg jakobrosenberg added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Jan 26, 2021
@ljharb
Copy link
Contributor

ljharb commented Jan 26, 2021

I would very much not expect it to do the same thing - when cc’d into the package, I’d expect npm install to work the same way it would as if the package was not in a workspaces repo at all.

@ljharb
Copy link
Contributor

ljharb commented Jan 26, 2021

In other words, the outer dir is a workspace, the inner dir is not. All repo commands should only be run from a project root.

@jakobrosenberg
Copy link
Author

Thanks for the clarification. 🙂

The inner dir is part of a workspace. If that changes depending on where I last ran npm install, everything becomes unpredictable.

I would expect the file structure to be the source of truth, It's predictable and logical. If I didn't want project1 to be part of myworkspace, I would not locate it within the workspace.

@ljharb
Copy link
Contributor

ljharb commented Jan 26, 2021

If that were true, what happens if I make /package.json and put workspace info in it? Would every project on my machine suddenly become part of a global workspace, and any npm install would run the installation in every project on my machine at once?

@jakobrosenberg
Copy link
Author

Yes, but creating a workspace in the root of your file system makes as much sense as running rm -rf in the root of your file system. Why would you do that in the first place, if not to break things?

Keeping workspaces contained in respective folders lets projects detect whether they're part of a workspace by letting them traverse down the tree and check for a package.json. It's simple, predictable and easy to reason about.

simpleproject/
myworkspace1/
  project1/
  project2/
myworkspace2/
  projecta/
  projectb/  

@ljharb
Copy link
Contributor

ljharb commented Jan 26, 2021

They'd have to traverse up the tree, which seems like a tricky thing to do.

@jakobrosenberg
Copy link
Author

jakobrosenberg commented Jan 26, 2021

Would this fit the bill?

const { sep, join } = require('path')
const multimatch = require('multimatch');

console.log(resolveWorkspacePath(__dirname))

/**
 * traverses back through the tree and returns the first workspace dir
 * that has a matching workspaces glob for this dir
 */
function resolveWorkspacePath(projectDir) {
    const dirs = projectDir.split(sep)
    while (dirs.length) {
        if (isInWorkspaceBlob(projectDir, join(...dirs)))
            return join(...dirs)

        dirs.pop()
    }
}

function isInWorkspaceBlob(projectDir, workspaceDir) {
    try {
        const { workspaces } = require(join(workspaceDir, 'package.json'))
        const globs = workspaces.map(glob => join(workspaceDir, glob))
        return multimatch(projectDir, globs).length
    } catch { }
}

@darcyclarke darcyclarke added Enhancement new feature or improvement Needs Discussion is pending a discussion Awaiting Information further information is requested and removed Needs Triage needs review for next steps Bug thing that needs fixing labels Feb 2, 2021
@raphaelsaunier
Copy link

I ran into the exact same issue after trying to install a new dependency in a workspace that already depends on another local package.

To illustrate the problem using by using @jakobrosenberg's initial scenario, it was after running cd workspace/project2 && npm install new-dependency (and not just npm install, but the result seems to be the same)

I created a sample repo to reproduce and isolate the problem before landing on this issue. Sharing it here in the hope that it can be useful to steer the discussion and resolve the problem or at least give better hints/warnings when running npm install in a workspace:

https://github.com/raphaelsaunier/npm-install-in-workspace-issue (I tried with both scoped and unscoped modules because at first I wrongly assumed it was related to scoped packages, before noticing that it affected both)

I'll let you discuss and figure out what's best, but if I may, here are two additional points to consider in the discussion:

  • If the dependency is added manually to project2/package.json, running npm install from the root will install it just fine. If, as @ljharb suggests, all repo commands have to be run from the root when using workspaces, I could live with that limitation but a more helpful error message and/or a notice in the docs about this significant caveat would greatly improve the DX because I'm fairly certain I won't be the only one bumping into this issue.
  • Yarn seems to handle this situation just fine and will let you run yarn add in a workspace and resolve any local dependencies with no errors.

@frangio
Copy link

frangio commented Jul 30, 2021

They'd have to traverse up the tree, which seems like a tricky thing to do.

I don't understand what is tricky about this. Node and npm traverse up the file system tree regularly for all sorts of common operations. Did I misunderstand the comment?

I think the command npm install <dependency> (as opposed to npm install) makes a better case for why commands should automatically detect a workspace and run relative to the root package.

To install a dependency in a workspace, the developer currently has to manually cd to the root package and run npm install <dependency> -w <workspace>. This is really unergonomic, and counter to what people are used to, both from pre-workspaces npm or Yarn.

I'm sure that the current behavior will lead to unexpected behavior and redundant lockfiles, and unless this is changed I will personally avoid using npm workspaces for the projects I maintain.

@jakobrosenberg
Copy link
Author

I think the command npm install (as opposed to npm install) makes a better case for why commands should automatically detect a workspace and run relative to the root package.

I agree. I didn't even know that installing dependencies in a workspace required special syntax. Could explain past issues.

@zackerydev
Copy link

This also causes some issues with the way Netlify handles mono repo support.
https://answers.netlify.com/t/npm-7-and-individual-netlify-toml-files/41579

You can see the overview I posted there - but making npm 7 context aware would go a long way in making this experience smoother for developers.

@jridgewell
Copy link

I hit this when installing deps inside my sub-package constantly, just like #2546 (comment). I usually cd directly inside one of the sub-packages so that I can have a smaller file tree, run tests easily, etc. I want npm to be smart enough to handle that I'm inside a sub-package without having to go to the root, do the operation, and return to my sub-package.

I can understand there may be some performance/semantics that tricky if we make npm aware of workspaces. Particularly, it'd force a full traversal to the root just to check if there's a workspace definition somewhere above the local package.json. However, we could get around that by having a special key in the sub-packages that indicates that it's a sub-package and not a normal package:

When npm finds the local package.json, it checks if "sub-package" field is true. If so, we need to traverse upwards to find the root to perform a workspaces-aware operation. If not, then we're at the root and we don't need to traverse anymore.

@frangio
Copy link

frangio commented Aug 10, 2021

I think it would be reasonable to stop upwards traversal as soon as a package-lock.json is found. I imagine this would reduce the performance hit, if there is one.

@ljharb
Copy link
Contributor

ljharb commented Aug 10, 2021

@frangio that assumes one is always present; none of my library packages, including ones in a monorepo, have one.

@zackerydev
Copy link

Couldn't the upwards traversal stop when it reaches a package.json with a workspaces flag?

Similar to how root: true works with ESLint?

@frangio
Copy link

frangio commented Aug 10, 2021

@ljharb In that case it would traverse either up to the root or some reasonable configurable default like $HOME.

Are you confirming that the concern here is the performance hit of doing this traversal?

@ljharb
Copy link
Contributor

ljharb commented Aug 10, 2021

@frangio no, I’m just saying that the presence of a lockfile can’t be relevant.

@jridgewell
Copy link

I just published a helper binary that will perform workspace-aware operations called @ampproject/npw. Running npw install inside of project2 will traverse to find the workspace root, and invoke npm install -w project2. Essentially, just type npw instead of npm when working in a workspace.

Couldn't the upwards traversal stop when it reaches a package.json with a workspaces flag?

That would only work if you were actually inside a workspace, but 99% of packages aren't a workspace. We don't want to force all of those packages to traverse to the root directory just to figure out there's no workspace root.

@justinfagnani
Copy link

Came here to look for or file this same issue. I really want to be able to run workspace-aware commands from anywhere within my monorepo. When I'm working my cwd can easily be in the repo root or in a subpackage and I want -w commands to behave the same everywhere. lerna with --scope does this well, fwiw.

Thanks for the npw helper other @jridgewell !

@penguinsAreFunny
Copy link

How do you handle different version dependencies of packages?

main.ts
packages
package-1
dependencies: math v1.0.0
package-2
dependencies: math v.2.0.0
package.json

I want to use packages in main, basically just a simlink to the packages. Npm right now uses some fancy module resolving leading to error for my packages as the versions do not match the dependency of the package itself.
On npm install in all packages´ npm install will be executed.
The packages themselfes without workspaces work fine and can be published (as they also should) from their paths. Using workspaces leads to errors on npm install from the base directory though.
For me this does not seem to be wanted. If I develop a package I dont want to care which dependencies other packages have as my package is a module itself and should be encapsuled, shouldn´t it?

I really have a hard time modularizing npm packages which I all want to publish but also want to develop without publishing and installing them all the time. Any suggestions on how to do that? Are workspaces even ment for that?

@kernwig
Copy link

kernwig commented Jan 31, 2022

I expect everyone new to NPM Workspaces breaks their dependencies by doing an npm install from a package, as we are all trained to do this from wherever we see a package.json file. Then dependencies start mysteriously failing and eventually we figure out we can't do that. Every so often, I still slip up, screwing up my dependencies and requiring that I delete all node_modules, package-locks, and reinstall. It's just annoying for me now, but a terrible first experience for every new developer.

@nlf
Copy link
Contributor

nlf commented Feb 8, 2022

this is fixed by #4372 which will be part of the next npm release, later this week.

@WangHansen
Copy link

Is it possible to keep the origin behavior with a flag like --ignore-workspace?

Consider the following case where I have a mono repo with few packages A, B and C, but one of the package(package C) is self-contained which means it doesn't depend on other packages (A or B).

Currently to build this package C, I need to install all the dependencies at the monorepo root level and then just use a small part of them to build package C.

The difficult part would be that if you go to the folder where package C is located, you can only see a package.json without a package-lock.json. So you still have to traverse up to find a lock file.

So maybe one solution can be install as if there is a monorepo, however, only install all the packages needed by C so that I can avoid install the unnecessary packages in this monorepo when building with C.

@lorenzogrv
Copy link

lorenzogrv commented Jun 22, 2022

@WangHansen

Avoiding install of unnecesary packages:

npm install -w path/to/C
# or
npm ci -w path/to/C

@trusktr
Copy link

trusktr commented Jul 10, 2023

How do we cd into a sub-project and then npm install in it as if it isn't in a workspace?

This is useful for testing projects that can be used as a template for new standalone projects that are not in a workspace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Information further information is requested Enhancement new feature or improvement Needs Discussion is pending a discussion Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

No branches or pull requests