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 exports the mergeProjectConfig utility method from the @rstest/core package, making it available for external use alongside the existing mergeRstestConfig function. The new function is a convenience wrapper that merges ProjectConfig objects (which represent individual project configurations within a multi-project setup) using the existing mergeRstestConfig implementation.
- Adds a new exported function
mergeProjectConfigthat wrapsmergeRstestConfigwith a type cast - Exports the new function from the main package entry point
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/config.ts | Implements the new mergeProjectConfig function that delegates to mergeRstestConfig with a type cast |
| packages/core/src/index.ts | Adds mergeProjectConfig to the public API exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const mergeProjectConfig = ( | ||
| ...configs: ProjectConfig[] | ||
| ): ProjectConfig => { | ||
| return mergeRstestConfig(...configs) as ProjectConfig; |
There was a problem hiding this comment.
The type cast from RstestConfig to ProjectConfig is potentially unsafe. Since mergeRstestConfig can include properties like reporters, pool, coverage, etc. that are explicitly omitted from ProjectConfig, the returned object may contain properties that violate the ProjectConfig type contract. Consider implementing a proper merge function that only handles ProjectConfig properties, or add runtime filtering to remove the omitted properties before returning.
| export const mergeProjectConfig = ( | |
| ...configs: ProjectConfig[] | |
| ): ProjectConfig => { | |
| return mergeRstestConfig(...configs) as ProjectConfig; | |
| // Helper to pick only ProjectConfig properties from an object | |
| function pickProjectConfigProps(obj: any): ProjectConfig { | |
| // List of allowed ProjectConfig keys. Update this list if ProjectConfig changes. | |
| const allowedKeys = [ | |
| 'root', | |
| 'include', | |
| 'exclude', | |
| 'testTimeout', | |
| 'testMatch', | |
| 'testEnvironment', | |
| 'globals', | |
| 'setupFiles', | |
| 'teardownFiles', | |
| 'transform', | |
| 'moduleNameMapper', | |
| 'extensionsToTreatAsEsm', | |
| 'watch', | |
| 'silent', | |
| 'bail', | |
| 'maxWorkers', | |
| 'snapshotFormat', | |
| 'snapshotSerializers', | |
| 'resolver', | |
| 'testSequencer', | |
| 'testPathIgnorePatterns', | |
| 'testRegex', | |
| 'testRunner', | |
| 'testURL', | |
| 'verbose', | |
| // Add more keys as needed if ProjectConfig changes | |
| ]; | |
| return Object.fromEntries( | |
| Object.entries(obj).filter(([key]) => allowedKeys.includes(key)) | |
| ) as ProjectConfig; | |
| } | |
| export const mergeProjectConfig = ( | |
| ...configs: ProjectConfig[] | |
| ): ProjectConfig => { | |
| const merged = mergeRstestConfig(...configs); | |
| return pickProjectConfigProps(merged); |
| export const mergeProjectConfig = ( | ||
| ...configs: ProjectConfig[] | ||
| ): ProjectConfig => { | ||
| return mergeRstestConfig(...configs) as ProjectConfig; | ||
| }; |
There was a problem hiding this comment.
The newly exported mergeProjectConfig function lacks test coverage. Since the repository has comprehensive tests for mergeRstestConfig in packages/core/tests/config.test.ts, consider adding tests for mergeProjectConfig to ensure it correctly merges ProjectConfig objects and doesn't inadvertently include properties that should be omitted from ProjectConfig.
Summary
export
mergeProjectConfigmethod.Related Links
Checklist