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

Support workspaces from package.json #2255

Open
boenrobot opened this issue Jan 6, 2020 · 28 comments · May be fixed by #6881
Open

Support workspaces from package.json #2255

boenrobot opened this issue Jan 6, 2020 · 28 comments · May be fixed by #6881
Labels
area: monorepo Everything related to the pnpm workspace feature good first issue Yarn compat Issues related to compatibility with Yarn

Comments

@boenrobot
Copy link

I have a project that is in a monorepo and uses yarn right now.

I'm considering migrating to pnpm for the monorepo, as well as all other projects I contribute to, as I have a small SSD as local storage (and with Windows...). However, it would be ideal if there was compatibility between yarn's workspaces and pnpm, so that we don't have to jump "all in" to it.

To this end, yarn allows packages in a monorepo to be defined as an array in package.json called "workspaces". I haven't checked if pnmp supports it, but from the absence in the docs, I'm assuming it doesn't.

Considering there's a separate file already, I guess one also has to consider what to do if both are present. I think it would be most intuitive if pnpm-workspace.yaml completely overrides package.json's workspaces if present (for backwards compatibility). There could also be a pnpm-workspace.yaml option on whether to change that to extend the package.json array instead.

@zkochan
Copy link
Member

zkochan commented Jan 6, 2020

There are some naming conflicts here. I don't know who is right but in pnpm a workspace is a set of projects. In Yarn, a set of workspaces is a project. So pnpm cannot read from a field called "workspaces". For pnpm, a field called projects would make sense.

But we could check the presence of that field and print an error message suggesting to create a pnpm-workspace.yaml with the patterns moved there

@zkochan zkochan added the area: monorepo Everything related to the pnpm workspace feature label Jan 6, 2020
@boenrobot
Copy link
Author

I'm not sure I understand the scenarios in which this distinction makes a difference.

I mean, in both cases, each package in the monorepo gets as few dependencies within it as possible, and the package manager ensures the packages within the monorepo can link to each other via normal specification (as if they were not part of the same monorepo).

@zkochan
Copy link
Member

zkochan commented Jan 6, 2020

I mean, in both cases, each package in the monorepo

Yarn calls these packages workspaces. pnpm calls them projects. The term "workspace" is used for a different purpose in pnpm. It is like a synonym of monorepo.

I created a poll about what people think about these terms: https://twitter.com/ZoltanKochan/status/1214223464363675649

@boenrobot
Copy link
Author

boenrobot commented Jan 6, 2020

But what is the difference in practice?

If the difference is in name only, and you'll always define the same set in both places, there's no reason to have two places. One could just be used as an alias to the other.

@zkochan
Copy link
Member

zkochan commented Jan 6, 2020

I have posted my thoughts. Let's see what others think

cc @pnpm/collaborators

@boenrobot
Copy link
Author

boenrobot commented Jan 21, 2020

Yarn calls these packages workspaces. pnpm calls them projects. The term "workspace" is used for a different purpose in pnpm. It is like a synonym of monorepo.

When I say "package" I mean just "a folder with a package.json in it". With that in mind,

in both cases, each [folder containing a package.json file] in the monorepo gets as few dependencies within it as possible, and the package manager ensures the [folders with package.json] within the monorepo can link to each other via normal specification (as if they were not part of the same monorepo).

The only difference currently is the way one tells the package manager the mappings between the value you would give in an import/require, and the corresponding [folder with a package.json in it]. In both cases, there are glob patterns that are supposed to match the [folders with package.json in them] from where the import/require name should be extracted, and mapped to that matched folder.

And the feature request here is for pnpm to support yarn's mappings in package.json as a possible alias, but still prefer its own settings if both mappings are defined. How exactly are these mappings called in pnpm's own settings vs in package.json is besides the whole point, as they do the same thing.

@zkochan
Copy link
Member

zkochan commented Jan 21, 2020

I think I would be fine with the next solution:

  1. we keep the pnpm-workspace.yaml to define the root of the workspace
  2. we add some new field to pnpm-workspace.yaml that will tell pnpm to reuse glob patterns from some other config. For instance, useLernaConfig: true and useYarnConfig: true.

@boenrobot
Copy link
Author

boenrobot commented Jan 21, 2020

That's still not a zero config solution though... It would be fine as an additional option, but doesn't quite address the compatibility with yarn.

If it's just about the word "workspaces" leaving a very bad taste in your mouth and wanting to encourage users to not use package.json like that, here's a compromise:

  1. If pnpm-workspace.yaml exists in the folder of the package.json being installed, assume that is the workspace root, and get the mappings from there.
    1.1. If there's useYarnConfig: true, merge the mappings from package.json#.workspaces with those defined by pnpm-workspace.yaml (prefer those in pnpm-workspace.yaml if present in both). Similarly for useLernaConfig: true.
  2. (Here's the zero config part...) If pnpm-workspace.yaml does not exist, check if package.json#.workspaces is defined.
    2.1. If it is, assume this is a workspace root, and take the mappings from package.json#.workspaces, and proceed. In that event, also emit a warning. The warning should prompt users to create pnpm-workspace.yaml with useYarnConfig: true if they want to get rid of the warning, or define an empty packages list in pnpm-workspace.yaml with useYarnConfig: false if they would like to opt out of this auto detection, and get the package.json treated as a normal package, not a workspace root.

In that way, one can easily try out their pipeline with pnpm, and once they verify it works, either make the project "package manager agnostic" by getting rid of the warning, or move entirely to pnpm by getting rid of package.json#.workspaces in favor of pnpm-workspace.yaml.

@zkochan
Copy link
Member

zkochan commented Jan 21, 2020

This is not only about "bad taste". Almost all commands check if they are executed in the context of a workspace. It means that each pnpm run will search for an additional file in the parent directories. And it will have to read every package.json in every parent directory to find a "workspaces" field. Maybe these additional filesystem operations are negligible but I don't know. It would have to be measured.

@boenrobot
Copy link
Author

boenrobot commented Jan 21, 2020

To minimize such a penalty, the package.json checks could be done after the pnpm-workspace.yaml searches.

Sure, this would encourage the creation of pnpm-workspace.yaml with an empty packages list, merely so that one can disable this search on upper levels, but the boost gained from that should be negligible in most developers' systems.

I for one would not mind waiting even 1s more for each command if it means not having to take a few minutes researching why the command doesn't "just work", and adding pnpm-workspace.yaml to fix it.

@zkochan
Copy link
Member

zkochan commented Jan 21, 2020

I can suggest the following. If pnpm install is executed in the root of the repository and the package.json in the current working directory contains the workspaces field, then pnpm can:

  1. automatically create a pnpm-workspace.yaml file with the necessary configuration.
  2. run the installation on all yarn workspace packages.

@boenrobot
Copy link
Author

boenrobot commented Jan 21, 2020

Yes, that would work too. The few extra bytes on HDD per monorepo are nothing compared to the space savings offered by pnpm.

Though it would be best if said config uses the previously suggested useYarnConfig: true, so that the mappings don't get out of sync.

@zkochan
Copy link
Member

zkochan commented Jan 21, 2020

Yes, that's what I meant. The autogenerated pnpm-workspace.yaml will contain useYarnConfig: true

@DesignByOnyx
Copy link

I am in full agreement that it would be nice if pnpm could piggyback on my current workspaces definition inside of package.json. I hate to prolong this discussion, but individual [things with a package.json file] inside of a yarn project are allowed to define a workspaces field, mostly for declaring nohoist options for that [thing with a package.json file]:

"workspaces": {
  "nohoist": ["react-native/**"],
}

So, here is the effective type declaration for the "workspaces" field:

workspaces: string[] | {
  packages?: string[],
  nohoist?: string[]
};

Furthermore, yarn has a hard requirement that the root package.json is set as private: true. So the logic for detecting the root of a yarn project should be along these lines:

  • is private: true
  • is workspaces present
  • is workspaces an array
    • true: IS YARN WORKSPACE
    • false: does workspaces have a packages property which is an array
      • true: IS YARN WORKSPACE
      • false: keep crawling

@leonardfactory
Copy link

leonardfactory commented Jun 7, 2020

Another point to keep pnpm-workspace.yaml file mandatory is other packages using this to check if it's a pnpm package, like @manypkg/get-packages. If i'm correct this is used by @changesets too. Not sure if other packages do the same.

Even if pnpm-workspace.yaml is mandatory, other packages may break thinking it's Yarn. I'm positive about this change, but it could be more breaking than originally thought.

@ExE-Boss ExE-Boss added the Yarn compat Issues related to compatibility with Yarn label Jun 21, 2020
@hinell
Copy link

hinell commented Nov 17, 2020

@leonardfactory Well I think other tools can identify the pnpm by using package-json#engines field. No need for extra file.

@yaquawa
Copy link

yaquawa commented Mar 30, 2021

npm 7 supports workspaces too, it really should be aligned with them.

@JamesRamm
Copy link

Since npm v7 seems to have adopted the same definition of workspace as yarn, reusing the same package.json fields, it would make sense and encourage adoption of pnpm if it were to at least support the same (even if pnpm-workspace.yaml were kept for more advanced features).

@tjx666
Copy link

tjx666 commented Jul 8, 2021

any progress?

@Eliav2
Copy link

Eliav2 commented Nov 3, 2021

I've also considered moving to pnpm and found out that it does not use the same convention as yarn and npm. I think pnpm should support the package.json.workspaces as this accepted by the other major package managers

@smolinari
Copy link

smolinari commented Nov 13, 2021

We are learning about this constraint and missing nohoist with PNPM and "workspaces" and now have to look to moving to Yarn as the alternative (and really don't want to, as we were very happy with PNPM). Not sure Yarn will also work in the Rush monorepo scenario we have and the new project we are trying to introduce (which needs nohoist, but will report here if it does.

Edit: Welp. Yarn or rather Rush with Yarn has other issues. Seems Rush doesn't support Yarn's workspaces and thus can't use Yarn's workspaces features. Buggers....

Edit2: We've gotten the package to work by installing dependencies of a particular sub-package in our package. It's ugly, but shows that somehow Webpack's module resolution doesn't jive at all with PNMP's methodologies (at least I couldn't get it to jive). 🤷 Oh, and I tried a number of the hoist config options that PNPM works with to no avail. I appreciate that they were there and gave me some hope to make this one package work, but alas, it didn't. I also know this frontend framework package is working on a future major version of its CLI, which will dump Webpack for Vite and will then support PNPM. Yay! I can't wait. 😁

Scott

@hinell
Copy link

hinell commented Nov 15, 2021

I think this thread is more than one year old now. If the backward-compatible features aren't implemented it's worth to fork it and collaborate to establish one. In modern software development backward compatibility is a kill feature. If one don't get it, his project is doomed to fail.

@zkochan
Copy link
Member

zkochan commented Nov 15, 2021

I suggested a solution here: #2255 (comment)

We received no pull requests implementing it.

If one don't get it, his project is doomed to fail.

Don't be ridiculous.

@smolinari
Copy link

Btw, I got my problem solved by manually hoisting dependencies i.e. installing them myself. It makes my project's package.json mofugly, but I can continue my work.

Scott

@ruifortes

This comment was marked as off-topic.

@hinell

This comment was marked as off-topic.

@MarouaneMan

This comment was marked as off-topic.

@zkochan
Copy link
Member

zkochan commented Mar 21, 2022

I don't see any constructive feedback here, so I am locking this to contributors only.

I have suggested a solution that will work #2255 (comment)

Contributions are welcomed.

@pnpm pnpm locked as too heated and limited conversation to collaborators Mar 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: monorepo Everything related to the pnpm workspace feature good first issue Yarn compat Issues related to compatibility with Yarn
Projects
None yet
Development

Successfully merging a pull request may close this issue.