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

Workspaces issue progress #3294

Closed
18 of 22 tasks
bestander opened this issue May 1, 2017 · 32 comments
Closed
18 of 22 tasks

Workspaces issue progress #3294

bestander opened this issue May 1, 2017 · 32 comments

Comments

@bestander
Copy link
Member

bestander commented May 1, 2017

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:

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

arcanis commented May 9, 2017

@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.

@bestander
Copy link
Member Author

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.

@cpojer
Copy link
Contributor

cpojer commented May 9, 2017

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.

@richburdon
Copy link

yarn help workspace

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

Returns 404

@bestander
Copy link
Member Author

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/.

@bestander
Copy link
Member Author

bestander commented Jul 18, 2017 via email

@donaldpipowitch
Copy link
Contributor

donaldpipowitch commented Jul 21, 2017

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

@bestander
Copy link
Member Author

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.

@donaldpipowitch
Copy link
Contributor

Thank you for your work ❤

@elderfo
Copy link

elderfo commented Jul 25, 2017

Are workspaces still under the experimental flag in the RC?

@bestander
Copy link
Member Author

bestander commented Jul 25, 2017 via email

@richburdon
Copy link

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

@bestander
Copy link
Member Author

Sorry, I am waiting my colleagues to give feedback on the blogpost, a draft is here yarnpkg/website#580

@akosyakov
Copy link

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.

@arcanis
Copy link
Member

arcanis commented Aug 7, 2017

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.

@svenefftinge
Copy link

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.

@Hypnosphi
Copy link

@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';

@svenefftinge
Copy link

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.

@BYK
Copy link
Member

BYK commented Aug 9, 2017

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.

@BYK BYK assigned BYK and unassigned bestander Aug 25, 2017
BYK added a commit that referenced this issue Aug 25, 2017
**Summary**

Closes #3294 by enabling workspaces by default.

**Test plan**

Existing tests, slightly modified.
arcanis pushed a commit that referenced this issue Aug 25, 2017
…4262)

* Update: Enable Workspaces by default but keep the option to turn off

**Summary**

Closes #3294 by enabling workspaces by default.

**Test plan**

Existing tests, slightly modified.

* Fix bugz

* Update install.js
@arcanis arcanis reopened this Aug 25, 2017
@donaldpipowitch
Copy link
Contributor

donaldpipowitch commented Sep 12, 2017

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/*"
    ]
}

@rarkins
Copy link
Contributor

rarkins commented Sep 12, 2017

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.

@donaldpipowitch
Copy link
Contributor

💡 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?

@ghost
Copy link

ghost commented Sep 20, 2017

Hi guys, I have problem with Ember: #4503 (same as #4081)

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

@gustaff-weldon
Copy link

@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.

@bestander
Copy link
Member Author

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

joaolucasl pushed a commit to joaolucasl/yarn that referenced this issue Oct 27, 2017
…arnpkg#4262)

* Update: Enable Workspaces by default but keep the option to turn off

**Summary**

Closes yarnpkg#3294 by enabling workspaces by default.

**Test plan**

Existing tests, slightly modified.

* Fix bugz

* Update install.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests