ember-source peerdependencies issue with ember-try - ember-release

[ctjhoa]

Hi,
I'm having issue in mulitple addons relying on ember-render-modifiers with the ember-release test

https://github.com/concordnow/ember-aria-tabs/runs/6172043274?check_suite_focus=true
https://github.com/concordnow/ember-sidenotes/runs/6161797973?check_suite_focus=true

It seems to be caused by https://github.com/emberjs/ember-render-modifiers/pull/58/files

@ef4 How are we supposed to fix that?

[chancancode]

Is the issue that when using the release channels, the package.json version is a pre-release version (something ending in -*) so it's not matched by the || 4?

[chancancode]

see also https://discord.com/channels/480462759797063690/568935504288940056/968497261766123530

[ef4]

@chancancode dependencySatisfies uses the includePrerelease flag to semver.satisfies(), so prereleases are allowed to match.

[ef4]

Oh, I see this is actually a question about NPM behavior, not dependencySatisfies.

The problem here is that the peerDep really isn't satisfied. Think about it this way: ember-render-modifiers is saying "any package that depends on me needs to also depend on ember-source". But ember-aria-tabs is violating that rule, which is why NPM is erroring.

In this case, the correct solution is for ember-aria-tabs to also list ember-source as a peerDependency. That establishes the chain of dependencies that we want: ember-render-modifiers will get the same copy of ember-source that ember-aria-tabs is getting, and in turn ember-aria-tabs will get the same copy of ember-source that its consumer (whether an app or another addon) is getting. As a result, NPM will guarantee that everybody is getting the same copy of ember-source.

Through years of bad, loose behavior, NPM has given a lot of people the wrong mental model for how dependencies actually work. There is nothing magic that would make the app's copy of ember-source available to ember-render-modifiers unless every step along the dependency chain between the app and ember-render-modifiers declares peer dependencies. Historically, you often got this behavior by accident, but now they are beginning to get more correct/strict about it.

If you have many intervening packages outside your control that are relying on the bad old behavior, that's what npm's --legacy-peer-deps option is for.

[ctjhoa]

@ef4 Sorry but I'm not following you on this.
If the issue was a missing peerDependency in those addons, all tests scenarios should have failed no?

And it's not the case here.
Only ember-release, ember-canary and ember-beta are failing

[ef4]

I see, you're right. The issue I mentioned about the missing peer dep is also a problem (you can see that even the passing cases print a warning about it) and will break people who use monorepos sometimes. But it is a separate issue.

So yes, NPM now enforces peer deps and applies semver ranges without the prerelease option. They insist this is by design even though it's extremely annoying for cases like this.

The argument is that it's "unsafe" to force another package to get a prerelease version when it didn't ask for it. But since we also have overrides for that purpose, I think overrides is a possible solution, see concordnow/ember-aria-tabs#428

That passes for me locally, it needs approval to run in CI.

[ctjhoa]

CI is all green, thanks for your contribution, I will apply the same fix on the other addon.