-
Notifications
You must be signed in to change notification settings - Fork 116
fix(testing-environment): should have Node on global
#1850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: ca4401c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
📝 WalkthroughWalkthroughBumps @testing-library/jest-dom to v6.9.0 across packages and templates, adds a Vitest test script in the testing-environment package, and updates the Vitest environment to assign and later delete Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CodSpeed Performance ReportMerging #1850 will degrade performances by 10.41%Comparing Summary
Benchmarks breakdown
Footnotes
|
React Example#5682 Bundle Size — 237.56KiB (0%).a9cd5ef(current) vs 8dd1ecb main#5673(baseline) Bundle metrics
|
| Current #5682 |
Baseline #5673 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
166 |
166 |
|
68 |
68 |
|
46.82% |
46.82% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #5682 |
Baseline #5673 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
91.8KiB |
91.8KiB |
Bundle analysis report Branch colinaaa:colin/0930/jest-dom Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#5677 Bundle Size — 365.13KiB (0%).a9cd5ef(current) vs 8dd1ecb main#5668(baseline) Bundle metrics
Bundle size by type
|
| Current #5677 |
Baseline #5668 |
|
|---|---|---|
239.33KiB |
239.33KiB |
|
93.8KiB |
93.8KiB |
|
32KiB |
32KiB |
Bundle analysis report Branch colinaaa:colin/0930/jest-dom Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/hot-gifts-carry.md(1 hunks)packages/react/testing-library/package.json(1 hunks)packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/package.json(1 hunks)packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/package.json(1 hunks)packages/testing-library/examples/basic/package.json(1 hunks)packages/testing-library/testing-environment/package.json(1 hunks)packages/testing-library/testing-environment/src/env/vitest/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/hot-gifts-carry.md
🧠 Learnings (1)
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
PR: lynx-family/lynx-stack#1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
packages/react/testing-library/package.json
🔇 Additional comments (7)
packages/testing-library/testing-environment/package.json (2)
53-53: LGTM: Test script addition.Adding a test script is appropriate for a testing environment package and enables direct test execution.
56-56: LGTM: Dependency upgrade to jest-dom v6.9.0.The version bump aligns with the PR's objective to support @testing-library/jest-dom v6.9.0, which requires the Node global exposure fix.
.changeset/hot-gifts-carry.md (1)
1-9: LGTM: Well-documented changeset.The changeset correctly documents the fix with appropriate patch version bumps for both affected packages and includes helpful references to the upstream jest-dom changes.
packages/testing-library/examples/basic/package.json (1)
18-18: LGTM: Consistent dependency upgrade.The version bump keeps the example package in sync with the testing environment upgrade.
packages/react/testing-library/package.json (1)
16-16: LGTM: Completes coordinated dependency upgrade.The version bump aligns with the other packages in this PR, completing the coordinated upgrade to jest-dom v6.9.0 across the testing stack.
packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/package.json (1)
19-19: LGTM! Version bump aligns with upstream changes.The update to @testing-library/jest-dom v6.9.0 is necessary to support the library's new global
Nodereference, which is addressed by the Vitest environment changes mentioned in the PR.packages/rspeedy/create-rspeedy/template-react-vitest-rltl-ts/package.json (1)
21-21: LGTM! Consistent version bump.The @testing-library/jest-dom update to v6.9.0 matches the change in the JavaScript template and supports the new global
Noderequirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/testing-library/testing-environment/src/env/vitest/index.ts (1)
16-16: LGTM: Correctly exposes Node to global scope.The assignment properly addresses the ReferenceError by exposing the DOM Node constructor from jsdom to the global scope, which @testing-library/jest-dom v6.9.0 requires. The cleanup in teardown (line 22) ensures no leakage between test runs.
Optional: Consider preserving pre-existing global.Node.
If
global.Nodealready exists before setup (unlikely in typical test environments), this assignment will overwrite it without restoration. Consider storing and restoring any pre-existing value if you want to be defensive.Apply this diff to preserve and restore any pre-existing
global.Node:async setup(global) { + const originalNode = global.Node; const fakeGlobal: { jsdom?: any; } = {}; await builtinEnvironments.jsdom.setup(fakeGlobal, {}); const lynxTestingEnv = new LynxTestingEnv(fakeGlobal.jsdom as JSDOM); global.lynxTestingEnv = lynxTestingEnv; global.Node = lynxTestingEnv.jsdom.window.Node; return { teardown(global) { delete global.lynxTestingEnv; delete global.jsdom; - delete global.Node; + if (originalNode !== undefined) { + global.Node = originalNode; + } else { + delete global.Node; + } }, }; },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/testing-library/testing-environment/src/env/vitest/index.ts(2 hunks)
🔇 Additional comments (3)
packages/testing-library/testing-environment/src/env/vitest/index.ts (3)
1-1: LGTM: Environment type import is correct.The addition of the
Environmenttype import is necessary for the type annotation on line 26 and follows idiomatic TypeScript practices.
22-22: LGTM: Cleanup resolves past review concern.The deletion of
global.Nodein teardown correctly prevents leaking the Node constructor between test runs, addressing the concern raised in the previous review.
26-26: LGTM: Proper use of TypeScript's satisfies operator.The
satisfies Environmentannotation provides compile-time type safety, ensuring the environment object conforms to Vitest's Environment interface without widening the type. This is the recommended pattern for type-checking object literals.
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/[email protected] ### Patch Changes - Add `event.stopPropagation` and `event.stopImmediatePropagation` in MTS, to help with event propagation control ([#1835](#1835)) ```tsx function App() { function handleInnerTap(event: MainThread.TouchEvent) { "main thread"; event.stopPropagation(); // Or stop immediate propagation with // event.stopImmediatePropagation(); } // OuterTap will not be triggered return ( <view main-thread:bindtap={handleOuterTap}> <view main-thread:bindtap={handleInnerTap}> <text>Hello, world</text> </view> </view> ); } ``` Note, if this feature is used in [Lazy Loading Standalone Project](https://lynxjs.org/react/code-splitting.html#lazy-loading-standalone-project), both the Producer and the Consumer should update to latest version of `@lynx-js/react` to make sure the feature is available. - Fix the "ReferenceError: Node is not defined" error. ([#1850](#1850)) This error would happen when upgrading to `@testing-library/jest-dom` [v6.9.0](https://github.com/testing-library/jest-dom/releases/tag/v6.9.0). - fix: optimize main thread event error message ([#1838](#1838)) ## @lynx-js/[email protected] ### Patch Changes - Bump Rsbuild v1.5.13 with Rspack v1.5.8. ([#1849](#1849)) ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`19f823a`](19f823a)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Fix the "ReferenceError: Node is not defined" error. ([#1850](#1850)) This error would happen when upgrading to `@testing-library/jest-dom` [v6.9.0](https://github.com/testing-library/jest-dom/releases/tag/v6.9.0). ## @lynx-js/[email protected] ### Patch Changes - feat: support load bts chunk from remote address ([#1834](#1834)) - re-support chunk splitting - support lynx.requireModule with a json file - support lynx.requireModule, lynx.requireModuleAsync with a remote url - support to add a breakpoint in chrome after reloading the web page - Updated dependencies \[]: - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support load bts chunk from remote address ([#1834](#1834)) - re-support chunk splitting - support lynx.requireModule with a json file - support lynx.requireModule, lynx.requireModuleAsync with a remote url - support to add a breakpoint in chrome after reloading the web page - Updated dependencies \[[`a35a245`](a35a245)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Updated dependencies \[[`a35a245`](a35a245)]: - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - feat: support load bts chunk from remote address ([#1834](#1834)) - re-support chunk splitting - support lynx.requireModule with a json file - support lynx.requireModule, lynx.requireModuleAsync with a remote url - support to add a breakpoint in chrome after reloading the web page - Updated dependencies \[[`a35a245`](a35a245)]: - @lynx-js/[email protected] - @lynx-js/[email protected] - @lynx-js/[email protected] ## @lynx-js/[email protected] ### Patch Changes - Avoid generating `.css.hot-update.json` when HMR is disabled. ([#1811](#1811)) ## [email protected] ## @lynx-js/[email protected] ## [email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] ## @lynx-js/[email protected] Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@testing-library/jest-domhas published a new version v6.9.0, which would reference theNodefrom global. See: testing-library/jest-dom#702Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests
Checklist