primitives: [DismissableLayer] Layering breaks with different component versions

Initially raised on Discord.

If two components (e.g. DropdownMenu and Dialog) are using different versions of DismissableLayer internally, then the layering breaks because of the different context instances.

We never added scoping to DismissableLayer since it should always cross component boundaries, however, I wonder if scoping could help here since it effectively passes context through a private prop that it expects the component to use. It might be worth looking into as a solution.

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Reactions: 4
  • Comments: 22 (7 by maintainers)

Commits related to this issue

Most upvoted comments

For all having this issue now:

Our current solution to this is using pnpm’s dependency override functionality.

Adding this to package.json forces everything to use DismissableLayer version 1.0.2.

"pnpm": {
	"overrides": {
		"@radix-ui/react-dismissable-layer": "1.0.2"
	}
},

This worked for me:

  1. Overriding the react-dismissable-layer version in package.json
  "resolutions": {
    "@radix-ui/react-dismissable-layer": "^1.0.5"
  }
  1. Deleting and reinstalling node_modules. It wasn’t working until I did this.

Hardcoding to "latest" instead of a said version would kinda solve that, but […] changes could be introduced and older versions of popover for example might not be compatible with those changes forced by the latest.

i had considered this but thought maybe changes to its apis could be handled with a deprecation warning. given that it’s a pretty stable comp at this point and not likely to change much, that might not be so bad. it seems better than the current state of things anyway where things break for folk.

I’m more worried about dependency changes, whilst the API of DismissableLayer might not change much, even a small change in any underlying dep (@radix-ui/primitive, @radix-ui/react-primitive for example) will generate a new version which will bring the issue again.

something like:

A component is using a deprecated DismissableLayer api. To solve this, upgrade your components to the same version number.

Also on this, “the same version number” is very hard to communicate as they’re independently versioned so unlikely to be aligned, so effectively you have to say, make sure the primitives that use DismissableLayer are all aligned in terms of its version. It’s very difficult… 😦

@StephenHaney the difference is that a globalThis solution means they will have different versions of DismissableLayer in their bundle still, so the chances of hitting this issue are the same (since useContext works based on referential equality), we’ve just improved the messaging.

to reduce the chances of hitting it, radix could remove the DismissableLayerContext altogether and useSyncExternalStore so all packages reference the same version of global state, however, that’d just move where breaking changes can occur—instead of them occurring when the consumer API changes (my proposal) they would occur when an implementation detail changes (the context shape/global store shape).

Radix usually treats context as internal so the expectation is that it is safe to change at will w/o introducing breaking changes. therefore, i’d be more concerned about that approach resulting in breaking changes for consumers. comparatively, props are always the public facing API and a change there is expected to be a breaking change—hence they rarely change.

Do I understand correctly: in your proposed solution, any version of a primitive would automatically grab the latest version of DismissableLayer (which is nice because older and newer primitives would work together) and then it’s on us to make sure we don’t make breaking changes to DismissableLayer?

yes your understanding is correct but you can make breaking changes. breaking changes wld be handled the same way they’re handled when props API changes for any of the comps (all packages bumped a major which yarn version handles automatically etc.). there’s no need to create a new package—radix can just log in dev mode if DismissableLayer sniffs an old version of its props being used.

type DismissableLayerProps {
- foo: boolean;
+ bar: boolean;
}
const DismissableLayer = props => {
   if (props.foo !== undefined && process.env.NODE_ENV = 'development') {
    // stack trace wld likely show which comp used this prop
    throw new Error('A component is using an outdated DissmissableLayer API. Please upgrade comp to latest');
   }
}

that’s what i meant by a smoother migration path bcos radix could just warn here initially and support both props for a little while.

We’re facing this issue in https://github.com/strapi/strapi. We use radix as a bedrock for https://github.com/strapi/design-system but because we’re extensible there appears to be some conflicts happening in specific projects.

We’re looking to recommending using resolutions as a workaround – but is there any appetite to investigate into solutions further? I appreciate it’s very difficult & I’m not gonna pretend I know where to start in solving it either 😅

Here’s how mobx handles cross-environment global for the purpose of detecting multiple versions of the package at runtime. Basically use globalThis where it’s supported and then fall back to individual environment global names. This approach would work in SSR and browser. react-dismissable-layer could register itself to a global in this way and, if it’s already there, throw an error.

The peerDependencies route seems promising as well but I think that would require the app author to install react-dismissble-layer manually in addition to the primitives?

Hardcoding to "latest" instead of a said version would kinda solve that, but […] changes could be introduced and older versions of popover for example might not be compatible with those changes forced by the latest.

i had considered this but thought maybe changes to its apis could be handled with a deprecation warning. given that it’s a pretty stable comp at this point and not likely to change much, that might not be so bad. it seems better than the current state of things anyway where things break for folk.

something like:

A component is using a deprecated DismissableLayer api. To solve this, upgrade your components to the same version number.

they would still have to solve it the way radix suggests today but this would rarely be necessary because the api hardly ever changes.

another benefit here is that radix would have control over and knowledge of when this issue arises so a migration path can be added to release notes when DismissableLayer api changes.

making it a peer dependency would surely suffer the same issues and add an extra step for consumer by requiring them to manage the dependency.