oddbird / oddbird/css-anchor-positioning
[BUG] Repeat `polyfill()` runs grow the inline `style` attributes
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 493
- Forks
- 18
- Avg merge
- 12h 37m
- Merged PRs (30d)
- 5
Description
Repeat
polyfill()runs grow the inlinestyleattributes, because we keep adding them. That isn't caused by this PR, but we've now amplified it since we're matching against more declarations.I think we could fix this by removing stale attributes that we've added on a previous run before we re-shift on a subsequent run. Something like this in
cascade.ts(I haven't tested this):diff --git a/src/cascade.ts b/src/cascade.ts --- a/src/cascade.ts +++ b/src/cascade.ts @@ -132,12 +132,31 @@ export function registerShiftedProperties( } } +/** + * Remove every declaration of `property` from `block`. + * + * Used before appending a generated declaration, so that re-processing a block + * we already wrote doesn't accumulate a duplicate each time. Inline styles are + * re-parsed from the `style` attribute a previous run wrote back, so this is + * the difference between an idempotent run and one that grows the attribute + * without bound. Removing rather than skipping keeps last-wins order when a + * block declares the same property twice. + */ +function dropExistingDeclarations(block: Block, property: string) { + block.children.forEach((child, item) => { + if (isDeclaration(child) && child.property === property) { + block.children.remove(item); + } + }); +} + /** * Shift property declarations for properties that are not yet natively * supported into custom properties. */ function shiftUnsupportedProperties(node: CssNode, block?: Block) { if (isDeclaration(node) && SHIFTED_PROPERTIES[node.property] && block) { + dropExistingDeclarations(block, SHIFTED_PROPERTIES[node.property]); block.children.appendData({ ...node, property: SHIFTED_PROPERTIES[node.property], @@ -162,6 +181,7 @@ function expandInsetShorthands(node: CssNode, block?: Block) { const appendProperty = (property: string, value?: CssNode) => { if (!value) return; + dropExistingDeclarations(block, property); block.children.appendData({ ...node, property,Or @jpzwarte and @jamesnw if you think this deserves its own issue and PR, that's fine too!
Originally posted by @jgerigmeyer in https://github.com/oddbird/css-anchor-positioning/pull/448#pullrequestreview-4928827142
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in src/cascade.ts, especially registerShiftedProperties, shiftUnsupportedProperties, and expandInsetShorthands. Reproduce repeated polyfill() processing with shifted properties and inset shorthands, then verify that previously generated inline declarations are removed before new ones are appended and repeated runs no longer grow the style attribute.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- css, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100