Fix bug in 'scoped' polyfill (BL-16510)#431
Conversation
Copied from node_modules/style-scoped/scoped.js as a baseline. This is the original Google polyfill with no modifications. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The original regex had a greedy `.*` after quoted values that could match across multiple attribute selectors, causing commas in comma-separated selector lists to be consumed incorrectly. Example of the bug: [class*="Ebook"].numberedPage:not(...), [class*="Device"]... The regex would match through both attributes, breaking selector separation. Changes: - Fixed attrRe to use lookahead assertion to stop at the closing bracket of the current attribute only - Enhanced consumeSelector to calculate startPos early and use it in all paths to skip leading whitespace properly - Changed .substr() to .substring() where appropriate to avoid deprecated API - Added .trimStart() to rest calculations to handle leading spaces after commas Includes comprehensive test cases for comma-separated selectors with attribute selectors and pseudo-elements. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
JohnThomson
force-pushed
the
fix/scoped-styles-attribute-selector-bug
branch
from
2f54e36 to
7fd10cb
Compare
The style-scoped npm package is no longer imported anywhere; we now use our own patched copy at src/scoped-styles-polyfill.js (which fixes the attribute-selector bug and is unlikely to receive upstream fixes since the package has been unmaintained since 2021). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
andrew-polk
left a comment
andrew-polk
left a comment
There was a problem hiding this comment.
@andrew-polk reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on JohnThomson).
src/scoped-styles-polyfill.test.ts line 4 at r1 (raw file):
// Mock the selector processing logic for testing function mockConsumeSelector(raw: string, prefix: string): { selector: string; rest: string } | null {
devin:
src/scoped-styles-polyfill.test.ts:R4-98
Tests duplicate the implementation rather than testing the actual polyfill module
The test file at src/scoped-styles-polyfill.test.ts re-implements consumeSelector and updateSelectorText as mockConsumeSelector and mockUpdateSelectorText rather than importing and testing the actual functions from src/scoped-styles-polyfill.js. This means the tests verify the mock's behavior, not the real polyfill's. If the real polyfill diverges from the mock (e.g., a typo in the regex or logic), the tests would still pass. This is understandable since the polyfill is an IIFE with no exports, but it means the tests provide weaker guarantees than they appear to.
2 participants
This change is