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)
For all having this issue now:
Our current solution to this is using pnpm’s dependency override functionality.
Adding this to
package.jsonforces everything to use DismissableLayer version 1.0.2.This worked for me:
react-dismissable-layerversion inpackage.jsonnode_modules. It wasn’t working until I did this.I’m more worried about dependency changes, whilst the API of
DismissableLayermight not change much, even a small change in any underlying dep (@radix-ui/primitive,@radix-ui/react-primitivefor example) will generate a new version which will bring the issue again.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
DismissableLayerare all aligned in terms of its version. It’s very difficult… 😦@StephenHaney the difference is that a
globalThissolution means they will have different versions ofDismissableLayerin their bundle still, so the chances of hitting this issue are the same (sinceuseContextworks based on referential equality), we’ve just improved the messaging.to reduce the chances of hitting it, radix could remove the
DismissableLayerContextaltogether anduseSyncExternalStoreso 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.
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 versionhandles automatically etc.). there’s no need to create a new package—radix can just log in dev mode ifDismissableLayersniffs an old version of its props being used.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
globalThiswhere it’s supported and then fall back to individual environment global names. This approach would work in SSR and browser.react-dismissable-layercould register itself to a global in this way and, if it’s already there, throw an error.The
peerDependenciesroute seems promising as well but I think that would require the app author to installreact-dismissble-layermanually in addition to the primitives?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:
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
DismissableLayerapi 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.