chore(deps): bump deps and update use-scroll-lock for jsdom 27.3+ CSSOM changes#242
chore(deps): bump deps and update use-scroll-lock for jsdom 27.3+ CSSOM changes#242
Conversation
sadmann7
commented
Jan 3, 2026
sadmann7
commented
- Bump deps
- Add data attributes to track scroll position for reliable testing
- Update test to check data-scroll-lock-top/left attributes
- Work around jsdom 27.3+ CSS parser issue with top/left properties
- All 18 tests passing
- Add data attributes to track scroll position for reliable testing - Update test to check data-scroll-lock-top/left attributes - Work around jsdom 27.3+ CSS parser issue with top/left properties - All 18 tests passing
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates project dependencies and implements a workaround for jsdom 27.3+ CSSOM changes that affect how CSS properties like top and left are reflected in the style object. The solution adds data attributes to track scroll position values for reliable testing while maintaining the existing style property assignments for real browsers.
Key Changes
- Updated multiple dependencies including jsdom (27.2.0 → 27.4.0), vitest (4.0.9 → 4.0.16), and various other packages
- Added
data-scroll-lock-topanddata-scroll-lock-leftattributes to store scroll position values - Refactored iOS scroll lock implementation to use individual style assignments instead of Object.assign
- Updated tests to verify data attributes instead of style properties to accommodate jsdom limitations
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updated dependencies including jsdom, vitest, and numerous other packages to their latest versions |
| packages/shared/src/hooks/use-scroll-lock.ts | Added data attributes for scroll position tracking and refactored iOS-specific scroll lock implementation |
| packages/shared/test/use-scroll-lock.test.tsx | Updated test to verify data attributes and added proper configuration for window.scrollX/scrollY properties |
| package.json | Updated dev dependency versions for testing libraries and tools |
| docs/package.json | Updated documentation-related dependencies including fumadocs packages and UI libraries |
Comments suppressed due to low confidence (1)
packages/shared/src/hooks/use-scroll-lock.ts:32
- The trailing comma is removed here, but in other parts of this PR trailing commas are being added (e.g., lines 162, 166). This creates an inconsistent code style. Consider either keeping or removing trailing commas consistently throughout the file.
style.overflow + style.overflowX + style.overflowY,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Store the scroll position values for restoration | ||
| body.setAttribute("data-scroll-lock-top", String(topValue)); | ||
| body.setAttribute("data-scroll-lock-left", String(leftValue)); | ||
|
|
||
| // Set top and left - try to apply even if JSDOM doesn't reflect them |
There was a problem hiding this comment.
While the data attributes are helpful for testing, the code still attempts to set style.top and style.left (lines 213-214). If jsdom 27.3+ doesn't properly reflect these properties in the style object, these assignments may be ineffective in the test environment. Consider either:
- Documenting that these style assignments are for real browsers while data attributes are for testing/restoration
- Using the data attributes as the source of truth for restoration instead of maintaining duplicate state
The current approach maintains both mechanisms but it's not clear which one is authoritative.
| // Store the scroll position values for restoration | |
| body.setAttribute("data-scroll-lock-top", String(topValue)); | |
| body.setAttribute("data-scroll-lock-left", String(leftValue)); | |
| // Set top and left - try to apply even if JSDOM doesn't reflect them | |
| // Store the scroll position values for restoration and testing. | |
| // These data attributes are the source of truth when unlocking, | |
| // especially in non-browser environments (e.g. JSDOM) where style | |
| // properties may not be reflected reliably. | |
| body.setAttribute("data-scroll-lock-top", String(topValue)); | |
| body.setAttribute("data-scroll-lock-left", String(leftValue)); | |
| // Apply the visual offset for real browsers. In JSDOM 27.3+ these | |
| // assignments may not be observable on the style object, so tests | |
| // should rely on the data attributes above rather than these values. |
| // Check data attributes since jsdom 27.3+ doesn't properly reflect top/left in style object | ||
| expect(document.body.getAttribute("data-scroll-lock-top")).toBe( | ||
| String(scrollY), | ||
| ); | ||
| expect(document.body.getAttribute("data-scroll-lock-left")).toBe("0"); |
There was a problem hiding this comment.
The test now only verifies the data attributes and not the actual style.top and style.left properties. This means the test won't catch if the style properties fail to be set in real browsers (non-jsdom environments). Consider adding additional assertions to verify both the data attributes and the style properties are set, or add a comment explaining why only data attributes are checked in the test environment.
