fix(popover): support non-writable polyfilled methods - #5188
Open
olemartinorg wants to merge 1 commit into
Open
Conversation
The OddBird popover polyfill installs configurable methods without marking them writable. Assigning shadow-DOM wrappers directly therefore throws during module initialization in environments such as JSDOM, preventing every consumer test that imports Designsystemet from collecting tests. Redefine the methods through their property descriptors and restore those original descriptors during hot reload. Exercise the polyfilled descriptor shape in the web test setup so native-browser test runs cover this failure.
🦋 Changeset detectedLatest commit: 76ea0e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
olemartinorg
marked this pull request as ready for review
August 13, 2026 14:52
olemartinorg
requested review from
Barsnes,
eirikbacker and
mimarz
as code owners
August 13, 2026 14:52
Contributor
|
Hi @olemartinorg! Thanks for reporting and fantastic you make PRs directly to us! 🌟 Really appreciate it 🙏 We have noticed this issue too, and already have a PR fixing it, patching the oddbird polyfill locally in our code, while we wait for oddbird/popover-polyfill#291 to be released |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The shadow DOM support added wrappers around
HTMLElement.prototype.togglePopover,showPopover, andhidePopoverusing direct assignment. That works for native browser implementations, but@oddbird/popover-polyfilldefines these methods as configurable and non-writable. Assigning to them throws aTypeErrorwhile the Designsystemet module is loading.We discovered this while upgrading Altinn from Designsystemet 1.18.0 through 1.19.0 as part of Altinn/altinn-studio#19766. In Altinn's JSDOM/Vitest suite, importing any Designsystemet React component failed with:
Because the exception happened during module initialization, 118 test suites failed before collecting their tests, blocking progress on the upgrade.
This change installs the wrappers with
Object.defineProperty, preserving the existing descriptors while explicitly making the replacement methods writable. Hot-reload cleanup now restores the complete original descriptors instead of assigning the function values back. The web test setup mirrors the non-writable descriptor shape from the polyfill so this compatibility path remains covered in browsers with native popover support.Downstream verification in Altinn removed the import-time exception, our tests pass again.
Checks performed:
packages/web.Checks
pnpm changesetif relevant)