-
-
Notifications
You must be signed in to change notification settings - Fork 17
feat: support rs.hoisted
#717
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
✅ Deploy Preview for rstest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
|
Summary
Testing
|
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.
Pull request overview
This PR adds support for the rs.hoisted function to the rstest testing framework, which allows users to create hoisted mock functions that can be accessed before module imports are resolved. The feature enables better mock setup patterns by hoisting mock definitions similar to how rs.mock calls are hoisted.
Key changes:
- Added
rs.hoistedAPI with type definitions and runtime implementation - Updated dependency versions including React 19.2.1 → 19.2.3, Rspack 1.6.6 → 1.6.7, and other related packages
- Added comprehensive e2e tests demonstrating the hoisted functionality
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updated lockfile with dependency version bumps (React, Rspack, rsdoctor, debug, qs packages) |
| packages/core/src/types/mock.ts | Added type definition for the new hoisted function to RstestUtilities interface |
| packages/core/src/runtime/api/utilities.ts | Added stub implementation for hoisted function; modified resetModules return value |
| packages/core/src/core/plugins/mockRuntimeCode.js | Added webpack runtime code for rstest_hoisted that executes the provided function |
| packages/core/LICENSE.md | Corrected magic-string repository URL (cosmetic fix) |
| e2e/mock/tests/mockHoist.test.ts | Enhanced test to verify @rstest/core can be accessed when imported late |
| e2e/mock/tests/hoisted.test.ts | New test file demonstrating rs.hoisted usage with mock functions |
| e2e/mock/src/sum.ts | Added exports for bar and foo to support new test scenarios |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
e2e/mock/tests/mockHoist.test.ts:22
- Callee is not a function: it has type number or undefined.
c('c');
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| resetModules: () => { | ||
| // The actual implementation is managed by the built-in Rstest plugin. | ||
| return rstest; | ||
| return {} as any; |
Copilot
AI
Dec 12, 2025
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.
The return value for resetModules has changed from return rstest; to return {} as any;, which breaks the return type consistency. According to the type definition in mock.ts, resetModules should return RstestUtilities to allow method chaining. All other similar methods in this file return rstest to maintain this pattern.
| return {} as any; | |
| return rstest; |
| hoisted: () => { | ||
| // The actual implementation is managed by the built-in Rstest plugin. | ||
| return {} as any; |
Copilot
AI
Dec 12, 2025
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.
The hoisted function implementation returns {} as any but the type signature indicates it should return type T which is the result of calling the provided function fn. This could lead to runtime errors if users rely on the return value. The implementation should call and return the result of fn(), similar to how the webpack runtime code handles it in mockRuntimeCode.js.
| hoisted: () => { | |
| // The actual implementation is managed by the built-in Rstest plugin. | |
| return {} as any; | |
| hoisted: <T>(fn: () => T): T => { | |
| // The actual implementation is managed by the built-in Rstest plugin. | |
| return fn(); |
| //#region rs.hoisted | ||
| __webpack_require__.rstest_hoisted = (fn) => { | ||
| return fn(); | ||
| }; |
Copilot
AI
Dec 12, 2025
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.
Missing //#endregion comment to close the rs.hoisted region. All other similar regions in this file (rs.unmock, rs.doUnmock, rs.requireActual, rs.mock, rs.mockRequire, rs.doMock, rs.doMockRequire, rs.reset_modules) properly close their regions with //#endregion comments for consistency.
| }; | |
| }; | |
| //#endregion |
| ### magic-string | ||
|
|
||
| Licensed under MIT license in the repository at git+https://github.com/Rich-Harris/magic-string.git. | ||
| Licensed under MIT license in the repository at https://github.com/rich-harris/magic-string.git. |
Copilot
AI
Dec 12, 2025
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.
This appears to be a fix to update the repository URL from "git+https://github.com/Rich-Harris/magic-string.git" to "https://github.com/rich-harris/magic-string.git" (note the lowercase username). While this correction improves accuracy, it's unrelated to the PR's stated purpose of adding support for rs.hoisted. Consider addressing this in a separate PR focused on license file maintenance.
| Licensed under MIT license in the repository at https://github.com/rich-harris/magic-string.git. | |
| Licensed under MIT license in the repository at https://github.com/Rich-Harris/magic-string.git. |
|
|
||
| it('@rstest/core should be accessible even if it is imported late, it is specifically hoisted', async () => { | ||
| // @ts-expect-error | ||
| foo('a', 'b'); |
Copilot
AI
Dec 12, 2025
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.
Callee is not a function: it has type string or undefined.
Summary
depends on web-infra-dev/rspack#12302.
Related Links
Checklist