peerDependencies are not handled correctly

[layershifter]

Hello there 👋

Recently in Fluent UI we noticed that there are complains about peerDependencies that we never noticed before (microsoft/fluentui#23697, microsoft/fluentui#23681, microsoft/fluentui#23639).

---

I created a small reproduction case (https://github.com/layershifter/midgard-yarn-problem) and indeed there are warnings about peerDependencies:

$ npx midgard-yarn-strict

[WARNING] Unmet peer dependency: @types/react-dom in @fluentui/[email protected] (parent: @fluentui/[email protected])
[WARNING] Unmet peer dependency: @types/react-dom in @fluentui/[email protected] (parent: @fluentui/[email protected])
[WARNING] Unmet peer dependency: react-dom in @fluentui/[email protected] (parent: @fluentui/[email protected])
[WARNING] Unmet peer dependency: scheduler in @fluentui/[email protected] (parent: @fluentui/[email protected])

This is extremely strange as pkg-test declared in a workspace has all required dependencies declared:

{
  "name": "pkg-test",
  "version": "1.0.0",
  "dependencies": {
    "@fluentui/react-avatar": "9.0.0-rc.10",
    "@types/react": "^17",
    "@types/react-dom": "^17",
    "react": "^17",
    "react-dom": "^17",
    "scheduler": "^0.23.0"
  }
}

Yarn v1, Yarn v3 & NPM do not emit any warnings at this moment. But they will if react, react-dom, scheduler or any other peer dependency will not be declared in pkg-test/package.json (regardless of where it is in the dependency tree):

warning "workspace-aggregator-00586a69-44fc-449f-8041-63e36c8b5446 > pkg-test > @fluentui/react-avatar > @fluentui/react-popover > @fluentui/[email protected]" has unmet peer dependency "react@>=16.8.0 <18.0.0".
warning "workspace-aggregator-00586a69-44fc-449f-8041-63e36c8b5446 > pkg-test > @fluentui/react-avatar > @fluentui/react-popover > @fluentui/[email protected]" has unmet peer dependency "react-dom@>=16.8.0 <18.0.0".
warning "workspace-aggregator-00586a69-44fc-449f-8041-63e36c8b5446 > pkg-test > @fluentui/react-avatar > @fluentui/react-popover > @fluentui/[email protected]" has unmet peer dependency "scheduler@^0.19.0 || ^0.20.0"

---

I also tried to trace scheduler and it's declared as peerDependency of @fluentui/[email protected]:

{ 
  "peerDependencies": {
    "@types/react": ">=16.8.0 <18.0.0",
    "@types/react-dom": ">=16.8.0 <18.0.0",
    "react": ">=16.8.0 <18.0.0",
    "react-dom": ">=16.8.0 <18.0.0",
    "scheduler": "^0.19.0 || ^0.20.0"
  }
}

https://github.com/microsoft/fluentui/blob/a569864246be0184a5a51421b3da4450deafb854/packages/react-components/react-context-selector/package.json#L38

---

The fix that was made to remove this warning - add scheduler as peerDependency to all packages in a dependency tree (microsoft/fluentui#23681): @fluentui/react-context-selector -> @fluentui/react-popover -> @fluentui/react-components. That does not seem correct to me. I would expect that pkg-test (a package from the example) will fail if scheduler is not listed in dependencies/devDependencies/peerDependencies, but not for errors in a tree (i.e. node_modules).

For example, if an application @fluentui/react-components which uses @fluentui/react-context-selector we should have an error about unmeet peer dependency (if an application not listing scheduler as a dependency or devDependency). However, we should not see an error about @fluentui/react-components not listing scheduler - an application is not responsible for enforcing strictness in downstream repos.

Currently it's unclear why should a package be responsible for forwarding its peerDependencies 🙃

[ling1726]

PNPM will also not throw errors when peer depenencies are not forwarded up the dep tree https://github.com/ling1726/pnpm-peer-deps FYI @sjwilczynski

[jcreamer898]

pnpm treats peers as regular dependencies from my understanding...

So, here's how I think I view things... Midgard Yarn Strict definitely is the most strict of all the above package managers, for better or worse.

If you look at the above image, I think the issue MYS has is that...

An app takes a dependency on @fluentui/react.

@fluentui/react has peerDeps on react, react-dom, @types/react and @types/react-dom.

Ok, cool so far, but @fluentui/style-utilities depends on @fluentui/theme, and here's where things get interesting.

@fluentui/theme also has peerDeps on react, react-dom, @types/react and @types/react-dom.

However, @fluentui/style-utilities doesn't implement them in the parent child sense.

What MYS sees as the solution here is to forward them as "peerDependencies" instead of implementing them as "dependencies".

That's just how the current MYS algorithm views the world.

We've investigated the idea of changing the algorithm a bit to look and see if any parent has correctly implemented the peers, and allowing them to point to the parent, but haven't gotten around to a very performant way to do that.

I'd very much love to have an open dialog and conversation around this as this is definitely a pain point for us too. :)

[layershifter]

However, @fluentui/style-utilities doesn't implement them in the parent child sense.

And this has perfect sense 🐱

To clarify, @fluentui/style-utilities does not use react or react-dom, so it should not depend on it either in dependencies or peerDependencies.

---

What MYS sees as the solution here is to forward them as "peerDependencies" instead of implementing them as "dependencies".

I would like to see an example there 🐱 In meantime, I can share some expectations on how it could work ⬇️

Let's use @fluentui/react-components as an example as the issue started with it. Unlike your example, I think that graph should start in a reverse order:

flowchart LR
 TOKENS("`@fluentui/tokens
    _peerDependencies: {}_
    `")
    THEME("`@fluentui/react-theme
    _peerDependencies: {}_
    `")
    MENU("`@fluentui/react-menu
    peerDependencies: {'react', 'react-dom'}
    `")
    SELECTOR("`@fluentui/react-context-selector
    peerDependencies: {'react', 'react-dom', 'scheduler'}
    `")
    COMPONENTS("`@fluentui/react-components`")

    classDef align text-align:left

    TOKENS:::align --> THEME:::align --> MENU:::align
    SELECTOR:::align --> MENU
    MENU --> COMPONENTS
    COMPONENTS --> APP[app-pkg]

Loading

The branch with @fluentui/tokens & @fluentui/react-theme can be ignored as it does not have dependencies.

The base principle:

• If package has matching dependencies in peerDependencies - they SHOULD satisfy the declared range
• If package has matching dependencies in dependencies - peerDependencies SHOULD be merged and bubble up
• If package has no matching dependencies in dependencies & peerDependencies - peerDependencies SHOULD bubble up

The example:

1. @fluentui/react-context-selector declares peerDependencies, they should be resolved on @fluentui/react-menu

• react is present is both package.json#peerDependencies, range matches, bubble up 🆙
• react-dom is present is both package.json#peerDependencies, range matches, bubble up 🆙
• scheduler is not present in @fluentui/react-menu#dependencies & @fluentui/react-menu#peerDependencies, should bubble up 🆙

2. @fluentui/react-menu declares peerDependencies + got scheduler, they should be resolved on @fluentui/react-components

• react is present is both package.json#peerDependencies, range matches, bubble up 🆙
• react-dom is present is both package.json#peerDependencies, range matches, bubble up 🆙
• scheduler is not present is @fluentui/react-components#dependencies & @fluentui/react-components#peerDependencies, should bubble up 🆙

3. @fluentui/react-components declares peerDependencies + got scheduler, they should be resolved on app-pkg

• react is present is app-pkg#dependencies ✅
• react-dom is present is app-pkg#dependencies ✅
• scheduler is not present is app-pkg#dependencies, break install 🔴