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

[Bug]: Transitive internal dependencies versions are too loose #4174

Open
1 task done
benjamind opened this issue Mar 13, 2024 · 4 comments
Open
1 task done

[Bug]: Transitive internal dependencies versions are too loose #4174

benjamind opened this issue Mar 13, 2024 · 4 comments
Labels
bug Something isn't working triage An issue needing triage

Comments

@benjamind
Copy link
Contributor

benjamind commented Mar 13, 2024

Code of conduct

  • 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

@benjamind benjamind added bug Something isn't working triage An issue needing triage labels Mar 13, 2024
@benjamind benjamind changed the title [Bug]: Transitive internal dependencies are too strict [Bug]: Transitive internal dependencies are not strict enough Mar 14, 2024
@benjamind benjamind changed the title [Bug]: Transitive internal dependencies are not strict enough [Bug]: Transitive internal dependencies versions are too loose Mar 14, 2024
@Westbrook
Copy link
Collaborator

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
Copy link
Contributor Author

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/action-menu@0.41.1:
    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/menu@0.41.1:
    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/menu@0.41.2:
    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
Copy link
Contributor Author

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
Copy link
Collaborator

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage An issue needing triage
Projects
None yet
Development

No branches or pull requests

2 participants