Yarn: Workspaces issue progress

Created on 1 May 2017  ·  32Comments  ·  Source: yarnpkg/yarn

This issue will link all related discussions and RFC.

Prior art (wrappers on top of yarn and npm):

RFC:

Pull requests:

Issues/discussions:

Implementation phase:

  • [x] (0.25) 1. Installing dependencies of all workspaces in the root node_modules
  • [x] (0.26) 2. Commands to manage (add/remove/upgrade) dependencies in workspaces from the parent directory
  • [x] (0.27) 3. Ability for workspaces to refer each other, e.g. jest-diff → jest-matcher-utils inside jest monorepo
  • [x] (0.27) 4. package-hoister for workspaces, e.g. when dependencies conflict and can't be installed on root level
  • [x] 5. Remove the experimental flag and turn on by default
  • [ ] 6. Command to publish all workspaces in one go (looking for a champion)
needs-discussion triaged

Most helpful comment

I intend to finish the post by the end of the week.

On Mon, Jul 17, 2017 at 8:32 PM Rich Burdon notifications@github.com
wrote:

Thanks very much @bestander https://github.com/bestander

Any idea of the timeline (for the docs, and the features to land?)

Very much looking forward to it.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/yarnpkg/yarn/issues/3294#issuecomment-315948390, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACBdWBLUb0kiMRl8FDmscPt1SE_alD5Bks5sPCdIgaJpZM4NNLga
.

All 32 comments

@bestander @cpojer I was thinking about the workspaces field. In the RFC we merged, we decided to use globbing to find all the package data (packages/**/*). I like the syntax, but won't this be an issue from a perf standpoint? It means that any cross-workspace command that we execute must first stat a lot of sub-directories, then read all the package.json files in order to extract the names. That's unless we implement some kind of cache, of course.

It could add up to the start time as Yarn would need to find and read multiple package.json files.
But for most cases I don't think it will be a problem, if glob find becomes slow people could be motivated to use less aggressive glob rule.
Or we could come up with a cache.

I think this should be fine from a perf-perspective for now. I expect the number of shared projects to be a small number (<100) in most cases. As @bestander said, large projects can also directly embed all the paths.

yarn help workspace

Visit https://yarnpkg.com/en/docs/cli/workspace for documentation about this command.

Returns 404

Workspaces is not going to be a CLI command but rather a configuration.
I am planning to post about it here https://yarnpkg.com/blog/ and have a section on different ways to link local projects here https://yarnpkg.com/en/docs/.

I intend to finish the post by the end of the week.

On Mon, Jul 17, 2017 at 8:32 PM Rich Burdon notifications@github.com
wrote:

Thanks very much @bestander https://github.com/bestander

Any idea of the timeline (for the docs, and the features to land?)

Very much looking forward to it.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/yarnpkg/yarn/issues/3294#issuecomment-315948390, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACBdWBLUb0kiMRl8FDmscPt1SE_alD5Bks5sPCdIgaJpZM4NNLga
.

Should this issue track all workspace related bugs? If yes, I created two issues --

Thanks, @donaldpipowitch.
I think this issue can be closed soon.
All workspaces-related bugs we tag with https://github.com/yarnpkg/yarn/labels/feature-workspaces.

BTW, thanks a lot for submitting issues with great repro steps, very much appreciated.

Thank you for your work ❤

Are workspaces still under the experimental flag in the RC?

Yes, still under experimental

On Mon, Jul 24, 2017 at 11:07 PM elderfo notifications@github.com wrote:

Are workspaces still under the experimental flag in the RC?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/yarnpkg/yarn/issues/3294#issuecomment-317639114, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACBdWHSnlGt_9sz2WgKWVb4016jM6y6Rks5sRYY8gaJpZM4NNLga
.

@bestander any update on your blog or instructions for using workspaces?

Sorry, I am waiting my colleagues to give feedback on the blogpost, a draft is here https://github.com/yarnpkg/website/pull/580

Fantastic.
(Moving --nohoist to separate issue).

Comments on blog:
1). might want to emphasize (and document) "private": true is required (I missed that on first try).
2). For neophytes like myself you might mention that you then call yarn install as normal (i.e., no need for a different command -- and that all other commands -- e.g., yarn upgrade should just work as expected).

Thanks again. Awesome work, really appreciated.

The decision in both lerna and yarn workspaces seem to be to hoist node packages to a common top level node_modules folder. While it works nicely, due to the default module resolution of traversing up parent folders, it also puts all modules into scope and leads to accidentally referencing code that you don't have a dependency on.
One could instead change the name of the top level hoisted modules folder, so that it is not considered and have explicit symbolic links to it for all dependencies (and transitive ones).

Such a change would also allow for more flexible project layouts, where e.g. workspaces don't need to sit in a common parent.

Why not just use pnpm, then?

More seriously, this is the sort of thing eslint-plugin-import is capable of validating. It only complicates lerna/yarn hoisting for a vanishingly small gain.

On Aug 7, 2017, at 06:40, Sven Efftinge notifications@github.com wrote:

The decision in both lerna and yarn workspaces seem to be to hoist node packages to a common top level node_modules folder. While it works nicely, due to the default module resolution of traversing up parent folders, it also puts all modules into scope and leads to accidentally referencing code that you don't have a dependency on.
One could instead change the name of the top level hoisted modules folder, so that it is not considered and have explicit symbolic links to it for all dependencies (and transitive ones).


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

The correctness and tooling are bad, e.g. VS code proposes bogus imports which should not be accessible. You are right, it can be identified and fixed with eslint-plugin-import for pure js projects (e.g. if you use typescript you are out of luck) and fixed afterward. But such errors could be eliminated at all without additional tooling if node_modules are not hoisted to the root.

Having the nice support of hoisting in all kind of tools does not sound as a vanishingly small gain. The current hoisting strategy makes use of node module resolution without respecting it by putting not only shared dependencies in the root but all. If it does respect then all tools around will play nicely automatically.

VS code proposes bogus imports which should not be accessible

Seems like an issue with VS Code then? They probably should use the package.json as source of truth rather than the filesystem (or at least use it to validate things).

But such errors could be eliminated at all without additional tooling if node_modules are not hoisted to the root.

Symlinks have their own set of issues, so that's unfortunately not really a solution here.

The current hoisting strategy makes use of node module resolution without respecting it by putting not only shared dependencies in the root but all.

I'm not sure I understand, would you mind detailing what you mean by that?

Seems like an issue with VS Code then? They probably should use the package.json as source of truth rather than the filesystem (or at least use it to validate things).

VS Code follows node module resolution that allows having parent directories with node_modules folder to define shared modules, looking in package.json is not enough.

Symlinks have their own set of issues, so that's unfortunately not really a solution here.

The suggestion of @svenefftinge does solve the issue using linking of dependencies, including transitive, from node_modules folder that does not belong to the root.

I'm not sure I understand, would you mind detailing what you mean by that?

There are 2 packages: Foo and Bar. Foo depends on X, Bar depends on X, Y. Without hoisting one can introduce a parent package and declare X to avoid duplicating dependencies but keeping Y in Bar. With the current hoisting everything go to the root, making Y accessible to Foo and breaking developer tools.

To fix it one should invent additional tools. An alternative is to fix hoisting by not putting non-shared dependencies in the root.

I don't know. The use cases you're talking about looks to me things that should be handled by those tools rather than by the package managers. Don't forget that each rule we add to specify how we install dependencies also decreases our ability to improve it and try out new things in later releases. That's why how the hoister works is not specified: it gives us leverage to improve it without breaking contracts.

We follow a semantic based on package.json, which I believe is common enough for it to be natively supported by most tools. If yours doesn't, then maybe it might be worth considering implementing it. Just my personal thoughts 😉

I don't know. The use cases you're talking about looks to me things that should be handled by those tools rather than by the package managers. Don't forget that each rule we add to specify how we install dependencies also decreases our ability to improve it and try out new things in later releases. That's why how the hoister works is not specified: it gives us leverage to improve it without breaking contracts.

VS code and other tools follow node module resolution as defined here, looking at package.json is just one of steps. I don't see why they should be fixed.

We follow a semantic based on package.json, which I believe is common enough for it to be natively supported by most tools. If yours doesn't, then maybe it might be worth considering implementing it. Just my personal thoughts 😉

Most of the tools follow this semantic that does not allow to access dependencies of siblings packages. What is allowed by hoisting and why it does not play nicely.

Most of the tools follow this semantic that does not allow to access dependencies of siblings packages. What is allowed by hoisting and why it does not play nicely.

The Node resolution algorithm has no knowledge whatsoever of the dependencies field of the package.json. How it is interpreted is left to the package manager implementations, and the common interpretation is "I promise that if you specify a dependency, you will be able to access it". Once again, we make no guarantee that you can't require a module that is not listed, and this is by design.

Hence why I say: if you want your tools to be aware of the relationships between your packages, then you have to read their dependencies fields and interpret them, you can't just rely on the node_modules directory being in a particular state. Otherwise, you will just end up losing information.

The package.json is a means for yarn or npm to produce the node_modules folder(s). All downstream tools, transpilers and loaders should rely on the same scoping rules, that is the node_module resolution strategy. In my projects I have typescript, vscode, webpack, node. They all use the same module resolution strategy and they all 'see' stuff that they should not, because the hoisting puts too much on their scope.

@svenefftinge How do you think, is it OK to import a second-level dependency directly in your code? Say you have a dependency to react in your package.json, and react depends on fbjs. So should your tools warn you if you do this?

import emptyFunction from 'fbjs/lib/emptyFunction';

Re-exported transitive dependencies are what yarn and npm do by default, so I wouldn't argue that they should not be added. That said, with the design I proposed it would be possible to optionally only link the explicit dependencies.

That said, with the design I proposed it would be possible to optionally only link the explicit dependencies.

Which adds another feature/option that we need to support with increased complexity, that's the point @arcanis is trying to make. It is the toolmakers' responsibility to protect against this one.

Thank you every on for this great feature. I there an issue where I can track discussion/progress for "nested workspaces"? We used this previously with Lerna and it was a really nice experience. It looked similar to this:

packages/
  - buttons/
    - examples/
      - simple/
      - group/
  - forms/
    - examples/
      - simple/
      - complex/
      - validation/
...

So basically: publishable packages on the first layer and examples for every package at the second layer (with private: true).

So basically:

{
    "private": true,
    "workspaces": [
        "packages/*",
        "packages/*/examples/*"
    ]
}

If anyone is interested, I've added yarn workspaces support to Renovate. It's an open source tool for keeping dependencies and lock files up-to-date which you can run self-hosted (installable via npm/yarn) or via the GitHub App I run, if your repository is on GitHub.

💡 This seems to work:

{
    "private": true,
    "workspaces": [
        "packages/*",
        "packages/*/examples/*"
    ]
}

I guess "nested" workspace means multiple package.json files containing the workspaces property than? And _not_ having nested packages inside a workspace?

Hi guys, I have problem with Ember: https://github.com/yarnpkg/yarn/issues/4503 (same as https://github.com/yarnpkg/yarn/issues/4081)

Ember thinks package is missing, but it's installed in the root dir and it doesn't search for it.

@bsvipas I think this is ember-cli and at least ember-dependency-checker-related. I suggest you bring the issue to ember-cli team attention.

Thanks everyone, I'll mark this issue as closed.
Please go ahead and raise issues for particular cases.

Was this page helpful?
0 / 5 - 0 ratings