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 edge cases in setup file resolution by refactoring the getSetupFiles function into a separate module with enhanced resolver capabilities. The changes enable support for pure ES modules and file:// protocol paths in setup file configurations.
Key Changes:
- Replaced Node.js
createRequireresolver with rspack's enhanced resolver to support ESM-first resolution with proper condition names - Added support for file:// protocol paths by converting them using
fileURLToPathbefore resolution - Moved
getSetupFilesto a separate module with dynamic imports to defer expensive rspack loading
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/utils/getSetupFiles.ts | New module with enhanced setup file resolver using rspack's ResolverFactory for ESM support |
| packages/core/src/utils/testFiles.ts | Removed old getSetupFiles implementation (moved to separate module) |
| packages/core/src/core/runTests.ts | Changed to dynamic import of getSetupFiles for lazy loading |
| packages/core/src/core/listTests.ts | Changed to dynamic import of getSetupFiles for lazy loading |
| e2e/setup/index.test.ts | Added test cases for file:// protocol and pure ESM setup files |
| e2e/setup/fixtures/package-name/rstest.fileProtocol.config.ts | Test config using import.meta.resolve with file:// protocol |
| e2e/setup/fixtures/package-name/rstest.esm.config.ts | Test config for pure ESM package resolution |
| e2e/setup/fixtures/package-name/test-setup-esm-fixtures/package.json | ESM-only package.json with "import" condition in exports |
| e2e/setup/fixtures/package-name/test-setup-esm-fixtures/index.js | Setup file for ESM test case |
| pnpm-lock.yaml | Added entry for new test fixture package |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const esmFirstResolver = new resolver.ResolverFactory({ | ||
| conditionNames: ['node', 'import', 'require'], | ||
| }); | ||
| const { path: resolvedPath } = esmFirstResolver.sync(rootPath, request); |
There was a problem hiding this comment.
The return value destructuring assumes resolver.sync always returns an object with a 'path' property. If the resolver returns undefined, null, or an object without a 'path' property, this will cause a runtime error. Add validation to check if the resolved result is valid before destructuring.
1 participant
Summary
fix resolve setupFile edge cases
Related Links
Checklist