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

feat(nm): Better workspace peer dependencies treatment #3305

Closed
wants to merge 7 commits into from

Conversation

larixer
Copy link
Member

@larixer larixer commented Aug 17, 2021

What's the problem this PR addresses?

When you use workspace peer dependencies with pnp linker and then switch the project to nm linker, the workspace peer dependencies contract might not be fulfilled, because of the limitations of Node resolution algorithm. It is to be expected. However, we CAN make the developer experience better in this case by detecting the cases when we hit limitations and providing warnings and guidance to the user on how to adjust his project minimally so that all workspace peer dependencies contracts were met.

How did you fix it?

First, I must explain how workspace peer dependencies work. There is one rule of Yarn 1.x and there are at least two rules of Yarn 2+ core for workspace peer dependencies:

  1. Yarn 1.x rule - workspace peer dependencies are inherited from root workspace. The nm linker currently supports this rule in order to be backward compatible with Yarn 1.x. In fact, because in Yarn 2.x we have nested workspaces, this rule is extended and the nm linker inherits peer dependencies from the closest workspace higher up in the file system hierarchy. So, as per this rule, the parent workspace for peer dependencies is IMPLICIT and lies higher up in the fs hierarchy. Yarn 1.x had only ONE dependency graph.
  2. Yarn 2+ has MULTIPLE dependency graphs, each graph starting from a workspace, the number of dependency graphs equals the number of workspaces in the project. As per this new model, the workspace peer dependencies can be resolved to multiple different versions, depending on the workspace from which the graph starts. Note, that the parent for peer dependencies is EXPLICIT, the user must declare child workspace as a dependency in parent workspace in order for peer dependencies to be inherited from the parent.
  3. Yarn 2+ has also peer dependencies with the default concept for all peer dependencies, not only for workspace ones. As per this concept if there is no explicit parent the peer dependency will be resolved to the version declared in dependencies for the same package name. If a workspace uses peer dependencies with the default technique, at a minimum of one time the default will be picked for the dependency graph starting at this workspace.

Ideally, the nm linker should check all the 3 rules above and report problems with the resulting install if any of the above assumptions cannot be met due to Node resolution algorithm limitations. This task is challenging both in the sense that the nm linker must operate on only ONE graph and that it must meet the same assumptions that are defined on MULTIPLE dependency graphs and in the sense of providing good explanations to the user.

Challenges list:

  • Prevent transitive dependencies taking place higher up the install tree and blocking way to workspace peer dependencies. Solved via: fix(nm): Maximizes chances to end-up with only top-level node_modules #3267
  • Better initial graph assembling to not lost track of workspace peer dependency resolutions - work in progress.
  • Verifying all 3 rules by tracing all workspace peer dependencies after hoisting and letting the user know via a warning if any assumption does not hold true
  • Detecting and explaining that rules cannot be met due to multiple parents providing different versions. Proposing to use --preserve-symlinks
  • Detecting and explaining that rules cannot be met due to hoisting limits used by the user

Super challenges:

  • Handling the case when the user inserts workspace deep inside the dependency graph via resolutions
  • Usage of cyclic peer dependencies between workspaces

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@larixer
Copy link
Member Author

larixer commented Mar 15, 2022

Many ideas in this PR have been superseded or need another iteration due to changes brought by:
#3438
Closing

@larixer larixer closed this Mar 15, 2022
@larixer larixer deleted the larixer/nm-workspace-peers-alignment-to-pnp branch March 15, 2022 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant