Skip to content

fix(core): stop restoring after a few rounds when another script keeps undoing it - #880

Merged
QuintonJason merged 12 commits into
mainfrom
fix/restore-loop-guard
Oct 19, 2026
Merged

QuintonJason merged 12 commits into
mainfrom
fix/restore-loop-guard

Conversation

@QuintonJason

@QuintonJason QuintonJason commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Description

Pine puts back what a DOM morph strips from its components:

A morph strips once, so each of these restores once. But another script that undoes every restore, such as its own MutationObserver stripping the same class, keeps both sides going forever. That all happens in microtasks, so the page never renders or takes input again.

This caps them. limitRestores wraps a restore that returns whether it wrote anything:

  • Counting: only restores that actually wrote something count.
  • Giving up: past 5 such writes before the next task, it stops and lets the other script win, rather than hang the page.
  • Warning: the first time it gives up, it logs one console.warn naming the element, for example "pds-box: stopped restoring its host, as another script keeps changing it. Use props instead."
  • Resetting: the count resets on the next task, through a MessageChannel message (with a timer as the fallback), so a later morph is still restored.

Where the cap applies

  • Host restores: keepHostStyling (every HostStylingKeeper component).
  • Stencil classes: keepPineHydrated, capped per element, so a script fighting one component doesn't stop the others being restored.
  • Trigger ARIA: pds-dropdown-menu and pds-popover. Opening or closing writes the ARIA directly, outside the cap, so they recover on their next open or close.
  • Where it lives: limitRestores sits in utils/keep-hydrated.ts, next to the other shared restore helpers. If the global script imported attributes.ts, the CDN loader would fetch that whole module.

Docs: the nine component pages that say their host state "comes back if something removes it" now add that a script that keeps undoing it wins after a few rounds, with a console warning.

Size: the component chunks measure 349.51 KB against main's 350 KB limit, so no change to the limit is needed.

Fixes DSS-359

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

How Has This Been Tested?

  • unit tests:
    • limitRestores: only writes count, it resets on the next task, it warns once with the element, and it passes arguments through.
    • keepPineHydrated: gives up on a component another script keeps stripping, warns once, and still restores the others.
  • e2e tests: a competing script fights pds-box's host, pds-dropdown-menu's trigger ARIA and pds-popover's trigger ARIA. The other script wins with one warning, and the page stays responsive. A pds-dropdown-menu in ordinary use logs no warning.
  • Full core suite: everything passes except the 4 pds-chip "subtle and solid sentiments" e2e tests, which fail on main too.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes

Note

Medium Risk
Touches global hydration and host-restore behavior used by many components; normal morph/restore paths should be unchanged, but edge cases with competing DOM scripts may leave Pine state or ARIA wrong until open/close or the next task.

Overview
Fixes a page-freeze bug where Pine’s MutationObserver-based restores (host classes/styles, Stencil hydrated classes, dropdown/popover trigger ARIA) could fight another observer that undoes each write in an endless microtask loop.

Introduces limitRestores in keep-hydrated.ts: only restores that actually write count toward a cap of 5 per task (reset via MessageChannel / setTimeout). After that, Pine stops restoring, logs one console.warn per element and restore kind, and leaves the competing script’s DOM state in place until something changes again. stencilClassesOf remembers Stencil scope classes so restores still work after repeated strips.

Wires the cap through keepHostStyling (HOST_RESTORE), keepPineHydrated (per-element), and trigger observers on pds-dropdown-menu and pds-popover. setAttributesIfChanged / restoreHostStyling now return whether they wrote, so no-op observer callbacks don’t burn the budget.

Docs on nine components note the give-up behavior; new unit and e2e tests cover responsiveness, single warnings, and normal use without false warnings.

Reviewed by Cursor Bugbot for commit d26171c. Bugbot is set up for automated code reviews on this repo. Configure here.

@QuintonJason
QuintonJason requested a review from a team as a code owner October 16, 2026 22:08
@QuintonJason QuintonJason added the ran-gauntlet Multi-agent review gauntlet has been run on this branch label Oct 16, 2026
@QuintonJason QuintonJason self-assigned this Oct 16, 2026
@github-actions github-actions Bot added bug Something isn't working package: core Changes have been made to the Core package labels Oct 16, 2026
@netlify

netlify Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for pine-design-system ready!

Name Link
🔨 Latest commit d26171c
🔍 Latest deploy log https://app.netlify.com/projects/pine-design-system/deploys/6ad63d5d551a5bb0d271fb2d
😎 Deploy Preview https://deploy-preview-880--pine-design-system.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@pixelflips pixelflips left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes, mostly on how the docs and JSDoc describe what happens after a restore gives up. The cap itself looks solid: only real writes count, the reset waits for a real task boundary, and ordinary use stays well under it.

Comment thread libs/core/src/components/pds-dropdown-menu/docs/pds-dropdown-menu.mdx Outdated
Comment thread libs/core/src/utils/keep-hydrated.ts Outdated
Comment thread libs/core/src/utils/keep-hydrated.ts Outdated
Comment thread libs/core/src/utils/keep-hydrated.ts Outdated
Comment thread libs/core/src/utils/keep-hydrated.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4ded55e. Configure here.

Comment thread libs/core/src/utils/keep-hydrated.ts
@QuintonJason
QuintonJason merged commit b73b842 into main Oct 19, 2026
27 checks passed
@QuintonJason
QuintonJason deleted the fix/restore-loop-guard branch October 19, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working package: core Changes have been made to the Core package ran-gauntlet Multi-agent review gauntlet has been run on this branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants