-
Notifications
You must be signed in to change notification settings - Fork 239
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
Define which dependencies are shared among workspace projects #375
base: main
Are you sure you want to change the base?
Conversation
If `a` declares a dependency on `foo`, and `b` does _not_ declare that | ||
dependency, then the code in `b` can call `require('foo')` and it will work | ||
without any errors in development. But when `b` is published, and | ||
subsequently installed as a standalone project, it fails to find the `foo` | ||
dependency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we may want to explicitly declare it as a non-goal to address "if foo
depends on c
, a
can require('c')
even though it doesn't declare that dependency"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that's covered more directly by --mode=isolated
.
2. A workspace that has a peer dependency should share the same instance of | ||
that peer dependency with all other workspace projects, unless | ||
explicitly marked for isolation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. A workspace that has a peer dependency should share the same instance of | |
that peer dependency with all other workspace projects, unless | |
explicitly marked for isolation. | |
2. A workspace that has a peer dependency should share the same instance of | |
that peer dependency with all other workspace projects that declare that peer dependency, unless | |
explicitly marked for isolation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be provided by --mode=isolated
, I'd prefer to leave it out of this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused; if they don't declare the peer dep, and it's not provided by the graph, then it shouldn't be available at all.
In this case I'm not talking about the "accidental" availability because a transitive dep included it; i'm saying that with two sibling projects A and B, A declaring a peer dep on something shouldn't suddenly make it available to B.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In isolated mode, it won't (assuming it isn't also an explicit root dependency). However, in hoisted mode, anything "shared" between workspaces is shared by virtue of being hoisted, rather than symlinked into place. So, yes, a peer dep of A
will be available to workspace B
whether it lists it as a dependency or not.
The only way I can see to add that constraint would be to say that we put the peer dep somewhere other than ./node_modules
and symlink it into all the workspaces that peer depend on it. That feels like we're then basically doing the same thing as isolated mode, but only halfway, and blurs the expectation that "in hoisted mode, shared deps are hoisted; in isolated mode, shared deps are symlinked".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right - that “hallway” thing is precisely what the rfc i plan to write does :-) it’s fine if the current hoisting mechanism causes this behavior, but i was hoping that this RFC would maybe be the RFC i need to write :-p
3. A workspace should not be able to load any package that it does not have | ||
an explicit dependency on, with the exception of dependencies declared | ||
explicitly at a level higher than them in the directory tree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. A workspace should not be able to load any package that it does not have | |
an explicit dependency on, with the exception of dependencies declared | |
explicitly at a level higher than them in the directory tree. | |
3. A workspace should not be able to load any package that it does not have | |
an explicit dependency on (direct, or possibly transitive), with the exception of dependencies declared | |
explicitly at a level higher than them in the directory tree. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
178001d
to
d02ad26
Compare
e3c23e5
to
c3427b6
Compare
c3427b6
to
c268966
Compare
This is ready for review and discussion. Removed the attractive nuisance of figuring out how to mark things as shared/isolated. If you want it isolated, make it a (non-peer) dep; if you want it hoisted, declare it at the top level. |
cc: @VincentBailly |
- Peer dependencies of multiple workspaces within the project will always | ||
be shared with one another, raising an `ERESOLVE` if this is impossible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is explicitly saying that if you have 100 child workspaces, and 1 declares react as a peer dep, the other 99 have access to that version of react, and can't peer-depend on a conflicting version of react? What if they non-peer-depend on one? (the latter case might be a non-issue, if the peer's availability is due to hoisting and the child's react dependency is not hoisted per the next bullet point)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, all peer deps across all workspaces would have to be resolvable (as they must be today). But non-peer deps would be nested within the workspace, so no relevant conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A workaround here (which also does work today) is to list a different version of the peerDep in the workspace's devDependencies
for use in development. This allows it to nest today, though it won't be default. With this proposal, its presence in devDependencies
would force it to nest.
It seems that yarn PnP encourages/allows a kind of "escape hatch" around hoisting/dep availability: listing something as optional in peerDependenciesMeta and not listing it at all in peerDependencies. I'm not specifically advocating for this feature, but I wanted to make sure that we leave design space for it if it becomes relevant wrt this RFC, shared mode, or isolated mode. The use case where this came up: |
Summary of discussion from open RFC call 2021-11-03 We're unlikely to add an escape hatch using @ljharb points out that this is a step towards a more sensible default behavior, but would still like to see special handling of peerDependencies in a way that is similar to isolated-mode. That can be an separate extension to this RFC or an alternative that includes all the same behaviors described here. In a nutshell that would mean:
So, for example, if several workspaces have @isaacs brought up implementation challenge that this will pose in cases where peer sets are partially overlapping, how to optimally resolve cases where an overlapping version could be found but it's not an exact match. (For example, one workspace wants Consensus seemed to be reached that it's worth doing this proposal first, since it poses no great implementation challenges, and provides some benefits. Even though it doesn't go as far as we might like, it's not contradictory to those other goals, so it could be explored later. Concerns about excessive duplication could be met by suggesting Brought up the escape hatch to shared peerDeps by listing the peer dep in Could also tie the shared peerDep behavior to workspace groups (ie, every declared workspace group shares a single peerDependencies set, which will conflict if not resolvable). This would make it explicit, at least, rather than relying on inferring the peer set boundaries from the stated peerDependencies in each workspace. It seems like this proposal, and the more advanced peerDependencies handling, would somewhat obviate the need for No easy solution was suggested for the concern about cases where the user wants a sibling workspace module to be loaded from the registry rather than by linking to the sibling. (But again, we could use workspace groups as an explicit group boundary here as well.) |
I have a requirement where I have all of my workspaces including a module. but in one of them, I don't want the dependency to be hoisted out of the local. It can still go into the root, but not for one workspace. Is the solution to use |
References