fix: support use rstest global APIs in external modules #554
Conversation
✅ Deploy Preview for rstest-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where rstest global APIs weren't accessible when used in external modules (like node_modules). The fix modifies the global API registration mechanism to properly expose rstest functions globally.
- Renamed
getGlobalApitoregisterGlobalApiand changed its behavior to register APIs directly onglobalThis - Moved the global API registration to occur before context creation
- Added comprehensive e2e tests to verify global APIs work correctly in external modules
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/runtime/worker/index.ts | Modified global API registration to use globalThis and moved registration timing |
| e2e/runner/test/runner.test.ts | Added new test case for global APIs in external modules |
| e2e/runner/test/fixtures/test-rstest-globals/package.json | Test fixture package configuration |
| e2e/runner/test/fixtures/test-rstest-globals/index.js | Test fixture demonstrating global API usage |
| e2e/runner/test/fixtures/globals.test.ts | Test file that imports and uses global APIs |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const registerGlobalApi = (api: Rstest) => { | ||
| return globalApis.reduce<{ | ||
| [key in keyof Rstest]?: Rstest[key]; | ||
| }>((apis, key) => { | ||
| apis[key] = api[key] as any; | ||
| // @ts-expect-error register to global | ||
| globalThis[key] = api[key] as any; | ||
| return apis; | ||
| }, {}); |
There was a problem hiding this comment.
The function returns an empty object but doesn't populate the apis accumulator. Since the function is now registering globals directly on globalThis, it should either return void or properly populate and return the apis object.
Copilot uses AI. Check for mistakes.
| const rstestContext = { | ||
| global, | ||
| console: global.console, | ||
| Error, |
There was a problem hiding this comment.
The removal of the spread operator that conditionally included global APIs makes the code cleaner, but the context object now lacks the rstest APIs that were previously included when globals was true. Verify that this doesn't break existing functionality that relies on these APIs being available in the context.
| Error, | |
| Error, | |
| ...(globals ? api : {}), |
Copilot uses AI. Check for mistakes.
Summary
support use rstest global APIs in external modules.
Related Links
Checklist