embroider-build/embroider

Embroider unable to resolve matching peer dependencies

lfloyd117 opened this issue · 6 comments

lfloyd117 commented

In work to convert our Ember apps to Embroider, I've come across an issue where the build errors with:
Some V1 ember addons are resolving as incorrect peer dependencies. This makes it impossible for us to safely convert them to v2 format. despite having matching versions.

my-app@0.0.0 -> @myOrg/my-addon@14.2.21
    sees peerDep ember-source@4.12.4
      at /path/to/monorepo/node_modules/.pnpm/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+template@1.4.0_webpack@5.92.1/node_modules/ember-source
    but my-app@0.0.0 is using ember-source@4.12.4
      at /path/to/monorepo/node_modules/.pnpm/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+te_d57u3ixfkr4qimpfqyd7ephogm/node_modules/ember-source

You can see they are both resolving ember-source at v4.12.4 however it doesn't connect them due to PNPM's structuring, having 2 copies in ./pnpm. I have gone through all of the suggested resolutions in the Embroider docs and nothing works (delete lockfile & start over, auto-install-peers=false, dedupe).

Speaking on the Discord this was brought up as a "wrong dependency graph" but this is how PNPM works and is working as intended. If previous build pipeline & various other Node tooling can tell that ember-source@4.12.4 == ember-source@4.12.4, then this seems to be an issue in Embroider.

NullVoxPopuli commented

What are the reproduction steps?

but this is how PNPM works and is working as intended.

Yes, but it's not pnpm's job to know that it's making a browser project' and so the dep graph must be correct for browser projects.

If previous build pipeline & various other Node tooling can tell that ember-source@4.12.4 == ember-source@4.12.4,

At different paths tho, bucause the resolve algorithm.
In node, this is harmless unless you rely on module state. There is no size concern in node. In the browser, it's very important that the dep graph is correct, because browsers and web apps are very size constrained.

then this seems to be an issue in Embroider.

Nay, previous build tools were unreliable due to incorrect dep graphs and had really hard to debug behaviors as a result.
This lead to tons of overrides/resolutions usage and issues that came from that.

I understand this is frustrating, but we have to follow specs so we can have a shared understanding of how things work.

I super appreciate you putting up a reproduction, and i'll check it soon!

lfloyd117 commented

What are the reproduction steps?

They are in the repo's README. Essentially just building my-app.

NullVoxPopuli commented

idk if it was pointed out yet, but there is why this message prints:

Some V1 ember addons are resolving as incorrect peer dependencies. 
This makes it impossible for us to safely convert them to v2 format.

There are two peer problems:

my-app@0.0.0 (dev)-> @blackmesa/my-addon-one@0.0.0 -> ember-concurrency@4.0.2
    sees peerDep ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+template@1.4.0_webpack@5.92.1/node_modules/ember-source
    but my-app@0.0.0 is using ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+te_d57u3ixfkr4qimpfqyd7ephogm/node_modules/ember-source

my-app@0.0.0 (dev)-> @blackmesa/my-addon-one@0.0.0 -> ember-data@4.6.6 -> ember-inflector@4.0.3
    sees peerDep ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+template@1.4.0_webpack@5.92.1/node_modules/ember-source
    but my-app@0.0.0 is using ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+te_d57u3ixfkr4qimpfqyd7ephogm/node_modules/ember-source

Your addon doesn't declare any peers, and it must because it consumes libraries which have declared peers on ember-source -- this means that you need to forward the peer declaration so that ultimately the app's copy of ember-source is used.

however, because you have ember-source as a devDependency, you now need to inject the addon.

See these pnpm issues for ergonomics around this:

I made a tool to help with the syncing issues: https://github.com/NullVoxPopuli/pnpm-sync-dependencies-meta-injected

The reason you now need to inject is because when you resolve a dependency, if any node_modules directory contains the dependency, that will be what is resolved first. And this is a problem only in monorepos because devDeps are often used for types -- but this is not ideal because it means the way resolve works is that the wrong dependency is chosen -- not the app's copy of the dependency (ember-source in this case, but it can literally be anything).

There are two ways to resolve this:

  • inject the dep (which omits devDeps, and treats your in-monorepo package as if it were installed from npm)
  • remove the devDep copy of the duplicate dependency -- this means you'd need to specify types from elsewhere -- perhaps via "types": ["../../some-place-else" ]

This gets me passed the wrong-dep-graph-errors and shows me an error in your repro -- the app will never start (probably fine -- actually starting the app wasn't the goal):

Directory not found: <repo>/apps/my-app/mirage

For Option 2, you need to turn off the "auto install peers" behavior that many package managers have, including pnpm.

However, even that is not enough because your addon's dependencies declare peers, and those peers are resolved before the app's -- so even though we can remove devDeps from the addon, our graph is still wrong.


my-app@0.0.0 (dev)-> @blackmesa/my-addon-one@0.0.0 -> ember-concurrency@4.0.2
    sees peerDep ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+template@1.4.0_webpack@5.92.1/node_modules/ember-source
    but my-app@0.0.0 is using ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+te_d57u3ixfkr4qimpfqyd7ephogm/node_modules/ember-source

my-app@0.0.0 (dev)-> @blackmesa/my-addon-one@0.0.0 -> ember-data@4.6.6 -> ember-inflector@4.0.3
    sees peerDep ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+template@1.4.0_webpack@5.92.1/node_modules/ember-source
    but my-app@0.0.0 is using ember-source@4.12.4
      at <.pnpm>/ember-source@4.12.4_@babel+core@7.24.9_@glimmer+component@1.1.2_@babel+core@7.24.9__@glint+te_d57u3ixfkr4qimpfqyd7ephogm/node_modules/ember-source

So, we can move these to peers as well.

Unfortunately, there is one error left, and I ran out of my timebox to debug this (and is usually why I go with the injected approach):

lfloyd117 commented

Okay, thanks. With many local addons that can feel frustrating to have to add to all our apps package.jsons but that makes sense.
I appreciate the explanation and the PR examples. Thanks for giving an in-depth overview of it.

By chance, can you use glob patterns for dependenciesMeta?

NullVoxPopuli commented

that can feel frustrating to have to add to all our apps package.jsons

yes, 100%.
When I have to do a repetitive task on many package.jsons, I've found it faster to write a small node script to do it for me. I've made bunch of utils to help me do monotonous repetitive tasks with less typing https://ember-apply.pages.dev/modules/packageJson

By chance, can you use glob patterns for dependenciesMeta?

You cannot -- would be a feature request to the pnpm project tho

  • 👍1