[Bug]: Transitive internal dependencies versions are too loose

[benjamind]

Code of conduct

• [X] I agree to follow this project's code of conduct.

Impacted component(s)

all

Expected behavior

We should be able to install arbitrary sets of SWC components using hard pinned versions, e.g. 0.41.1 such that all internal dependencies thereby brought in by those components will internally resolve to not cause conflictings SWC component versions.

Actual behavior

When installing the following set of dependencies:

"@spectrum-web-components/menu": "0.41.1",
"@spectrum-web-components/action-menu": "0.41.1"

Its possible for package resolution to result in a duplicate install of @spectrum-web-components/menu at 0.41.2.

This is caused by the internal dependency in action-menu on picker with the semver ^0.41.1 being specified. This allows 0.41.2 to be installed for picker which in turn brings in menu at ^0.41.2. This second transitive dependencies on menu is then more restrictive than the menu installed by the app and thus we end up with two versions in the repository.

Suggest making internal dependencies use the 0.41.1 form of semver (with no ^) to ensure that when you install a component at a specific version you only get packages at that version installed. This will avoid unexpected installation of newer versions which can cause such conflicts.

Screenshots

No response

What browsers are you seeing the problem in?

No response

How can we reproduce this issue?

1. Add @spectrum-web-components/menu and @spectrum-web-components/action-menu to a project with a fixed version, e.g. 0.41.1
2. Install using pnpm
3. Inspectpnpm-lock.yaml and confirm that two versions of menu are installed.

Sample code that illustrates the problem

No response

Logs taken while reproducing problem

No response

[Westbrook]

In this situation did pnpm dedupe (as see here https://opensource.adobe.com/spectrum-web-components/registry-conflicts/#with-npm-or-pnpm) not prevent the duplication of dependencies? My understand is that would resolve up, but should flatten to the top of the available semver value. I'm scrambling for alternatives as the move to dependencies being listed with a ceiling and floor amounts to every version being breaking, regardless. It might not be a bad idea, but I want to make sure we've exhausted our alternatives, first.

[benjamind]

Sadly doesn't appear to. Here's a simple reproduction case:

{
  "name": "swc-test",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "@spectrum-web-components/action-menu": "0.41.1",
    "@spectrum-web-components/menu": "0.41.1"
  }
}

Installing this and then pnpm dedupe does nothing, and if you inspect the pnpm-lock.yaml you get some interesting results, here's some snippets from the pnpm-lock.yaml:

We get an action menu at 0.41.1 depending on 0.41.2 base and other components:

  /@spectrum-web-components/[email protected]:
    resolution: {integrity: sha512-UFCabpm6G5Z6GSilncjniiLYfVYkxmud2lDe30QC3pddoXSSiR3YSN9rHFxGXRddAghTHG4RfCIt+YaT4PMmjQ==}
    dependencies:
      '@spectrum-web-components/action-button': 0.41.2
      '@spectrum-web-components/base': 0.41.2
      '@spectrum-web-components/icon': 0.41.2
      '@spectrum-web-components/icons-workflow': 0.41.2
      '@spectrum-web-components/picker': 0.41.2
      '@spectrum-web-components/shared': 0.41.2
    dev: false

And we get duplicate menu components:

  /@spectrum-web-components/[email protected]:
    resolution: {integrity: sha512-tjNaE3gLrgchFIEjmNxERJC6TgEKhZv4TRxcGVRZlXZDKho+Ju7EMwIfzfoVAMtPN74CQzJrvFUY3QuuQsOgjQ==}
    dependencies:
      '@lit-labs/observers': 2.0.2
      '@spectrum-web-components/action-button': 0.41.2
      '@spectrum-web-components/base': 0.41.2
      '@spectrum-web-components/divider': 0.41.2
      '@spectrum-web-components/icon': 0.41.2
      '@spectrum-web-components/icons-ui': 0.41.2
      '@spectrum-web-components/overlay': 0.41.2
      '@spectrum-web-components/popover': 0.41.2
      '@spectrum-web-components/shared': 0.41.2
    dev: false

  /@spectrum-web-components/[email protected]:
    resolution: {integrity: sha512-KLwqEnVZIH69E99x4//hiDLUjspwMyM6IENlHLiGoL+8nHwnof88XOPguU5+N2v3zJCVMwF0audVFS+FnwjeXA==}
    dependencies:
      '@lit-labs/observers': 2.0.2
      '@spectrum-web-components/action-button': 0.41.2
      '@spectrum-web-components/base': 0.41.2
      '@spectrum-web-components/divider': 0.41.2
      '@spectrum-web-components/icon': 0.41.2
      '@spectrum-web-components/icons-ui': 0.41.2
      '@spectrum-web-components/overlay': 0.41.2
      '@spectrum-web-components/popover': 0.41.2
      '@spectrum-web-components/shared': 0.41.2
    dev: false

Of course if I change the package.json to depend on ^0.41.1 then things work as expected in that you end up with actually having 0.41.2 versions but at least they're all consistent. Problem is that multiple teams now have I think started to avoid loose pinning entirely and are going to fixed versions which is where this behavior becomes problematic for web components.

[benjamind]

Looking around for alternative approaches the only options I can find are:

• Clients must specify strict dependencies on everything - i.e. we got duplicated 0.41.2 of menu because picker was indirectly included by action-menu@0.41.1 via the picker@^0.41.1 dependency. So adding picker@0.41.1 in the consumers package.json this prevents this by constraining picker to 0.41.1.

  • Not ideal since you need to know the transitive dependencies of everything you include and hard pin them yourselves

• Clients need to use 'resolutions' even within a single package

  • Not ideal, its weird behavior to find this occur in such a simple include list.

• Clients should never use strict dedependencies and should instead use ^0.41.1 or 0.41.x style semver to allow everything to update to a later minor.

  • Not bad overall, but still feels odd that we have to force clients to use specific semver selections in order for components transitive dependencies to behave.

• Clients should use resolution-mode time-based in their .npmrc - https://pnpm.io/npmrc#resolution-mode

  • Really odd, but works well, though I don't think this is easily followed by all parties.

[Westbrook]

This is super helpful research. We'll need a little time to process. I get a bad feeling this is a not everyone can be happy thing, but trying to make those people as small a group as possible is what we'll try for!

[estollnitz]

I'd like to second the need for a fix to this issue. Here's our scenario (at Adobe):

• My team builds and publishes a package containing a custom web component that relies on some SWC components.
• Another team builds a web app that relies on my team's custom web component as well as other SWC components.
• Both teams explicitly specify that all SWC dependencies should use exactly version 0.41.1 to avoid any mismatched versions or duplicated component registrations.
• When run, the web app fails to load and reports duplicate web component registrations because both 0.41.1 and 0.41.2 are present.
• Both teams are mystified because the problem occurs seemingly at random — when SWC releases a patch version, and not when we make a code change. Dedupe does not help. The only thing that helps is to specify pnpm overrides in the web app's package.json.
• This pattern repeats for any other web apps that use my team's package.

One simple fix appears to be changing readPackageJsonNameVersion in scripts/lint-versions.js to use an exact version rather than a version prefixed with ^.

Westbrook, you mentioned that this would effectively make every change a breaking change. That's true only in the sense that it would prevent projects that use an exact version number of a component from picking up any patches to transitive dependencies. Using an inexact version number would allow patches, but only if all SWC dependencies (direct and transitive) increase at the same time — which is exactly the purpose of having synchronized version numbers in SWC. I don't really see any disadvantages.