fix: Refactor useMotionValueRefCore into a dedicated file, introduc…#2204
fix: Refactor useMotionValueRefCore into a dedicated file, introduc…#2204Yradex merged 1 commit intolynx-family:mainfrom
useMotionValueRefCore into a dedicated file, introduc…#2204Conversation
🦋 Changeset detectedLatest commit: f7c0497 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
291110e to
306c0a9
Compare
📝 WalkthroughWalkthroughCore hook logic was moved to a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/motion/__tests__/dependencies.test.ts`:
- Around line 22-30: The forbidden-file check currently compares the raw import
specifier `file` against `forbiddenFile` before path resolution, allowing
bypasses (e.g. `animation/index`, `animation/index.js`); move the forbidden
check to after resolution where `filePath` is computed (the block that resolves
`file` into `filePath` in the traversal logic), and perform the comparison
against the resolved path (normalize relative to `rootDir` the same way the
error message builds). Also change the `visited` set to use the resolved
`filePath` (not the raw `file`) so visits and forbiddens are consistent, and
ensure `pathStack` entries used in the thrown error are the resolved/normalized
paths for accurate diagnostics.
🧹 Nitpick comments (3)
packages/motion/src/mini/polyfill.ts (1)
4-22: Naming inconsistency:shimQueueMicroTaskvsqueueMicrotask.The function name uses capital
TinMicroTaskwhile the standard API and all other references use lowercaset(queueMicrotask). Consider renaming toshimQueueMicrotaskfor consistency.packages/motion/__tests__/mini-polyfill.test.tsx (1)
9-54: Test validates no-crash but not polyfill functionality.The test confirms the mini module doesn't throw in a main-thread environment, which is the stated goal. However, the
onCompletecallback (lines 26-29) doesn't capture or assert anything, so the test doesn't verify that the polyfill'squeueMicrotaskactually works for async scheduling. Consider adding an assertion that the animation completes (e.g., checkingmv.get()equals 100 after completion) in a follow-up if deeper coverage is desired.packages/motion/__tests__/dependencies.test.ts (1)
17-18: Consider adding an explicitexpectassertion for test clarity.The test relies solely on thrown errors for failure, which works but may trigger "no assertions" warnings depending on vitest config. A trailing
expect(e.g.,expect(true).toBe(true)) or importingexpectand using it aftercheck()would make the intent more explicit. Low priority.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
306c0a9 to
581eeba
Compare
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes
|
Web Explorer#7565 Bundle Size — 383.74KiB (0%).f7c0497(current) vs fe5e578 main#7562(baseline) Bundle metrics
|
| Current #7565 |
Baseline #7562 |
|
|---|---|---|
154.88KiB |
154.88KiB |
|
35.06KiB |
35.06KiB |
|
0% |
40.36% |
|
8 |
8 |
|
8 |
8 |
|
239 |
239 |
|
16 |
16 |
|
2.99% |
2.99% |
|
4 |
4 |
|
0 |
0 |
Bundle size by type no changes
| Current #7565 |
Baseline #7562 |
|
|---|---|---|
252.82KiB |
252.82KiB |
|
95.85KiB |
95.85KiB |
|
35.06KiB |
35.06KiB |
Bundle analysis report Branch f0rdream:motion_mini_seperation Project dashboard
Generated by RelativeCI Documentation Report issue
…e a `queueMicroask` polyfill for the `mini` package, and add dependency tests for the `mini` bundle.
581eeba to
f7c0497
Compare
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @lynx-js/motion@0.0.3 ### Patch Changes - Fix an issue that motion/mini will accidentally imports full version, causing compiling error ([#2204](#2204)) ## @lynx-js/react@0.116.3 ### Patch Changes - fix: remove `lynx.createSelectorQuery` deprecated warning in production ([#2195](#2195)) - Add a DEV-only guard that detects MainThread flush loops caused by re-entrant MTS handlers. ([#2159](#2159)) This typically happens when a MainThread handler (e.g. event callback or `MainThreadRef`) performs UI mutations (like `Element.setStyleProperty`, `setStyleProperties`, `setAttribute`, or `invoke`) that synchronously trigger a flush which re-enters the handler again. - Avoid DEV_ONLY_SetSnapshotEntryName on standalone lazy bundle. ([#2184](#2184)) - Add alog and trace for BTS event handlers. ([#2102](#2102)) - fix: Main thread functions cannot access properties on `this` before hydration completes. ([#2194](#2194)) This fixes the `cannot convert to object` error. - Remove element api calls alog by default, and only enable it when `__ALOG_ELEMENT_API__` is defined to `true` or environment variable `REACT_ALOG_ELEMENT_API` is set to `true`. ([#2192](#2192)) - fix: captured variables in main thread functions within class components do not update correctly ([#2197](#2197)) ## @lynx-js/rspeedy@0.13.4 ### Patch Changes - Bump ts-blank-space v0.7.0 ([#2238](#2238)) - Bump Rsbuild v1.7.3 with Rspack v1.7.5. ([#2189](#2189)) - Updated dependencies \[]: - @lynx-js/web-rsbuild-server-middleware@0.19.8 ## @lynx-js/qrcode-rsbuild-plugin@0.4.5 ### Patch Changes - Only register console shortcuts when running in TTY environments ([#2202](#2202)) ## @lynx-js/react-rsbuild-plugin@0.12.8 ### Patch Changes - Updated dependencies \[[`4240138`](4240138)]: - @lynx-js/react-webpack-plugin@0.7.4 - @lynx-js/react-alias-rsbuild-plugin@0.12.8 - @lynx-js/use-sync-external-store@1.5.0 - @lynx-js/react-refresh-webpack-plugin@0.3.4 - @lynx-js/template-webpack-plugin@0.10.3 ## @lynx-js/testing-environment@0.1.11 ### Patch Changes - Remove element api calls alog by default, and only enable it when `__ALOG_ELEMENT_API__` is defined to `true` or environment variable `REACT_ALOG_ELEMENT_API` is set to `true`. ([#2192](#2192)) ## @lynx-js/web-constants@0.19.8 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-worker-rpc@0.19.8 ## @lynx-js/web-core@0.19.8 ### Patch Changes - fix: avoid error when LynxView is removed immediately after connected ([#2182](#2182)) - Updated dependencies \[]: - @lynx-js/web-constants@0.19.8 - @lynx-js/web-mainthread-apis@0.19.8 - @lynx-js/web-worker-rpc@0.19.8 - @lynx-js/web-worker-runtime@0.19.8 ## @lynx-js/web-core-wasm@0.0.3 ### Patch Changes - Updated dependencies \[[`e0972ef`](e0972ef), [`3bc017e`](3bc017e), [`e2d349e`](e2d349e), [`d924b6a`](d924b6a)]: - @lynx-js/web-elements@0.11.2 - @lynx-js/web-worker-rpc@0.19.8 ## @lynx-js/web-elements@0.11.2 ### Patch Changes - Add scrollHeight/scrollWidth getters to XList. ([#2156](#2156)) - Inherit padding styles for x-input elements. ([#2199](#2199)) - Remove the default lazy-loading attribute from x-image elements. ([#2186](#2186)) - Fix x-input number type forwarding to the inner input element. ([#2193](#2193)) ## @lynx-js/web-mainthread-apis@0.19.8 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.19.8 ## @lynx-js/web-worker-runtime@0.19.8 ### Patch Changes - Updated dependencies \[]: - @lynx-js/web-constants@0.19.8 - @lynx-js/web-mainthread-apis@0.19.8 - @lynx-js/web-worker-rpc@0.19.8 ## @lynx-js/react-webpack-plugin@0.7.4 ### Patch Changes - Remove element api calls alog by default, and only enable it when `__ALOG_ELEMENT_API__` is defined to `true` or environment variable `REACT_ALOG_ELEMENT_API` is set to `true`. ([#2192](#2192)) ## create-rspeedy@0.13.4 ## @lynx-js/react-alias-rsbuild-plugin@0.12.8 ## upgrade-rspeedy@0.13.4 ## @lynx-js/web-core-server@0.19.8 ## @lynx-js/web-rsbuild-server-middleware@0.19.8 ## @lynx-js/web-worker-rpc@0.19.8 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Refactor
useMotionValueRefCoreinto a dedicated file, introduce aqueueMicrotaskpolyfill for theminipackage, and add dependency tests for theminibundle.Summary by CodeRabbit
Bug Fixes
New Features
Tests
Chores
Checklist