adobe / spectrum-web-components

Spectrum Web Components
https://opensource.adobe.com/spectrum-web-components/
Apache License 2.0
1.29k stars 205 forks source link

Component installations may need manual version resolutions #1228

Closed hunterloftis closed 3 years ago

hunterloftis commented 3 years ago

Expected Behaviour

The latest version of all components depend upon the latest versions of all other components, avoiding duplicate registrations of the same component via multiple included versions.

Should this be expected?

It's been working so far, but maybe that's not a guarantee the library makes.

Actual Behaviour

Without overriding transitive dependencies in package.json via resolutions, some combinations of components are incompatible.

The multiple installed versions lead to Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry': the name ${tagName} has already been used with this registry errors.

Steps to Reproduce

create a package.json:

{
  "name": "test-swc-versions",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "@spectrum-web-components/accordion": "*",
    "@spectrum-web-components/action-group": "*",
    "@spectrum-web-components/action-menu": "*",
    "@spectrum-web-components/actionbar": "*",
    "@spectrum-web-components/avatar": "*",
    "@spectrum-web-components/button": "*",
    "@spectrum-web-components/card": "*",
    "@spectrum-web-components/dialog": "*",
    "@spectrum-web-components/dropdown": "*",
    "@spectrum-web-components/field-label": "*",
    "@spectrum-web-components/icon": "*",
    "@spectrum-web-components/icons": "*",
    "@spectrum-web-components/icons-workflow": "*",
    "@spectrum-web-components/menu": "*",
    "@spectrum-web-components/overlay": "*",
    "@spectrum-web-components/popover": "*",
    "@spectrum-web-components/slider": "*",
    "@spectrum-web-components/switch": "*",
    "@spectrum-web-components/tabs": "*",
    "@spectrum-web-components/textfield": "*",
    "@spectrum-web-components/theme": "*",
    "@spectrum-web-components/toast": "*",
    "@spectrum-web-components/tooltip": "*",
    "@spectrum-web-components/top-nav": "*"
  }
}

install packages with yarn, then run:

$ yarn why @spectrum-web-components/popover
yarn why v1.22.5
[1/4] Why do we have the module "@spectrum-web-components/popover"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "@spectrum-web-components/popover@0.8.0"
info Has been hoisted to "@spectrum-web-components/popover"
info Reasons this module exists
   - Specified in "dependencies"
   - Hoisted from "@spectrum-web-components#action-menu#@spectrum-web-components#picker#@spectrum-web-components#popover"
info Disk size without dependencies: "168KB"
info Disk size with unique dependencies: "756KB"
info Disk size with transitive dependencies: "11.09MB"
info Number of shared dependencies: 14
=> Found "@spectrum-web-components/actionbar#@spectrum-web-components/popover@0.7.1"
info This module exists because "@spectrum-web-components#actionbar" depends on it.
info Disk size without dependencies: "156KB"
info Disk size with unique dependencies: "744KB"
info Disk size with transitive dependencies: "11.08MB"
info Number of shared dependencies: 14
=> Found "@spectrum-web-components/dropdown#@spectrum-web-components/popover@0.7.1"
info This module exists because "@spectrum-web-components#dropdown" depends on it.
info Disk size without dependencies: "156KB"
info Disk size with unique dependencies: "744KB"
info Disk size with transitive dependencies: "11.08MB"
info Number of shared dependencies: 14
Done in 0.25s.

and:

$ yarn why @spectrum-web-components/overlay
yarn why v1.22.5
[1/4] Why do we have the module "@spectrum-web-components/overlay"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "@spectrum-web-components/overlay@0.9.0"
info Has been hoisted to "@spectrum-web-components/overlay"
info Reasons this module exists
   - Specified in "dependencies"
   - Hoisted from "@spectrum-web-components#popover#@spectrum-web-components#overlay"
   - Hoisted from "@spectrum-web-components#tooltip#@spectrum-web-components#overlay"
   - Hoisted from "@spectrum-web-components#action-menu#@spectrum-web-components#picker#@spectrum-web-components#overlay"
info Disk size without dependencies: "432KB"
info Disk size with unique dependencies: "4.43MB"
info Disk size with transitive dependencies: "10.93MB"
info Number of shared dependencies: 13
=> Found "@spectrum-web-components/dropdown#@spectrum-web-components/overlay@0.8.2"
info Reasons this module exists
   - "@spectrum-web-components#dropdown" depends on it
   - Hoisted from "@spectrum-web-components#dropdown#@spectrum-web-components#popover#@spectrum-web-components#overlay"
info Disk size without dependencies: "424KB"
info Disk size with unique dependencies: "4.01MB"
info Disk size with transitive dependencies: "6.52MB"
info Number of shared dependencies: 6
=> Found "@spectrum-web-components/actionbar#@spectrum-web-components/overlay@0.8.2"
info Reasons this module exists
   - "@spectrum-web-components#actionbar#@spectrum-web-components#popover" depends on it
   - Hoisted from "@spectrum-web-components#actionbar#@spectrum-web-components#popover#@spectrum-web-components#overlay"
info Disk size without dependencies: "424KB"
info Disk size with unique dependencies: "4.01MB"
info Disk size with transitive dependencies: "6.52MB"
info Number of shared dependencies: 6
Done in 0.24s.

You can also see this with yarn list, which will show the multiple versions of both "overlay" and "popover."

Because of the multiple versions, customElements.define will be called multiple times by different versions of overlay/popover, and the second invocation of each will fail. (bundle sizes will also be increased)

Platform and Version

$ uname -a && node -v && yarn -v
Linux e92910339d40 5.8.0-44-generic #50~20.04.1-Ubuntu SMP Wed Feb 10 21:07:30 UTC 2021 x86_64 GNU/Linux
v14.15.4
1.22.5

Workarounds

As long as the components pointing to older versions of component dependencies are compatible with the newer versions, you can force the newer versions to be installed everywhere via resolutions in package.json. For example:

"resolutions": {
    "**/@spectrum-web-components/popover": "0.8.0",
    "**/@spectrum-web-components/overlay": "0.9.0"
  }
Westbrook commented 3 years ago

Yes, this is a situation where I wish we had a better answer for consumers. This situation arises from the fact that package managers count ever feature (0.x.0) release pre-1.0.0 as breaking and assume you can't upgrade across those versions. With that assumption comes the possibility of supplying you with sub-dependencies that mismatch. In some projects, the worst problem that causes is duplicated code for you consumers, for web components that can cause the Uncaught DOMException: Failed to execute 'define' on 'CustomElementRegistry' issue you've encountered here.

You resolutions workaround is one approach to managing this, when working with yarn. Other workarounds with yarn include:

A more "wholistic" workaround would be to rely on @spectrum-web-components/bundle so that you only ever have to bump one dependency, and always have any and all SWC packages available. The main, and possibly serious, downside of this is that it runs afoul of some bundler tooling that requires you have a direct dependency on any package you directly import.

THE answer to this situation is for us to get past 1.0.0 in the versions for all of our packages so that semver and package managers can once again do their jobs and resolve all of these things on their own. We're attempting to solidify what would be needed to make this try either at the full project level or at the individual package level via https://github.com/adobe/spectrum-web-components/projects/1 and https://github.com/adobe/spectrum-web-components/issues/865 where we would absolutely love your insight into what would make these packages "stable" and/or "complete" from your perspective. As you'll notice from the conversations to date, the main areas of concern at this moment are:

In that long term world you shouldn't need to "install" the latest version of things, the latest version of what's needed for you project will be resolved to, and we'd like to get to a place of guaranteeing that as soon as it viable. In the meantime, we leverage CHANGELOG.md files in each individual package so that you can make an educated decision as to what you'd like to upgrade or not. Writing this now, it makes me thing that I should confirm how projects resolve that data and whether or not we should be including the CHANGELOG.md files in our publications of not. We're also looking at making a centralized CHANGELOG.md or other form of library wide change listing available in the near future to make consuming that data less difficult.

Knowing this situation, is there anything else that we could be doing/documenting/etc at the library level to further support you finding success in your work?

hunterloftis commented 3 years ago

Thanks for the thoughtful & comprehensive reply!

So far, I've been following the second pattern you mentioned (yarn upgrade) - I used "*" here to see if looser requirements might resolve the disparate versions. I first ran into this after executing:

"upgrade:swc": "yarn upgrade --latest --pattern '@spectrum-web-components/*'"

TIL about --scope. Also TIL about @spectrum-web-components/bundle - I'll try that next since it sounds like exactly what I need.

Regarding v1.0, my experience sketching out a reasonably complex UI with SWC has been brilliant. It's already as stable and unsurprising as any of the alternatives I've tried (material, ionic, various in-house component libraries). It's unusually internally-consistent, and aspects that I honestly didn't expect to work out-of-the-box (theming, adapting global dimensions for mobile, a11y-by-default) just did.

In my work with it, I've come up with several nice-to-haves, but none are "1.0"-level:

My main 1.0 ask is already covered in #1189. A non-select-stateful menu is just too common of a control not to have.

Westbrook commented 3 years ago

Excellent to hear, we’ll keep charging forward and have a solid plan to share soon then.

FYI: In https://opensource.adobe.com/spectrum-web-components/components/slider/api check out getAriaValueText which is a property that accepts a function that is given the value of the slider as an argument and writes the output to the value side of the label. This is a great reminder to document that. Enjoy!

hunterloftis commented 3 years ago

Oh, another thing I ran into that seems like a "1.0" component (so much so that I think it's probably there and I'm missing it) is a non-dropdown select. The native select + option combination is severely restricted in style. Initially, I expected sp-menu to fill that role, with the selectable attribute leading to checkmarked items, but it doesn't appear to:

https://webcomponents.dev/edit/br27MZF5BQMA7wEGWjW5/src/index.ts

Westbrook commented 3 years ago

Check out some ongoing work outlined by https://github.com/adobe/spectrum-web-components/issues/1189 I’ll pass this feedback to @adixon-adobe who's working on that, and I think the work therein might empower this.