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.
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; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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(); | ||
| }; |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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'); |
There was a problem hiding this comment.
Callee is not a function: it has type string or undefined.
Summary
depends on web-infra-dev/rspack#12302.
Related Links
Checklist