fix: should not load real implementation when module is mocked#934
fix: should not load real implementation when module is mocked#934
Conversation
Summary of ChangesHello @9aoy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of preventing real module implementations from loading when a module is mocked, which is crucial for isolating tests and avoiding unintended side effects. The changes involve modifying how rs.mock and rs.doMock interact with Webpack's module loading mechanism, primarily by prioritizing the mocked module factory and delaying the loading of the original module until explicitly required for features like spying. New test cases have been added to validate this behavior, including scenarios with side effects and unmocking.
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical issue where rs.mock() was loading the real module implementation before applying the mock, causing unwanted side effects to execute.
Changes:
- Modified
rs.mock()to check the module cache first and avoid loading modules unless necessary (e.g., for spy/mock options) - Updated module factory handling to set factories instead of directly modifying cache, ensuring consistent behavior
- Enhanced webpack runtime protection to prevent mocked module factories from being overwritten during chunk loading
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/core/plugins/mockRuntimeCode.js | Refactored rs.mock() to check cache before loading modules, preventing side effects. Updated rs.doMock() to use module factories consistently. |
| packages/core/src/core/plugins/mockRuntime.ts | Added checks for rstest_original_module_factories to prevent webpack from overwriting mocked modules during chunk loading. |
| e2e/mock/tests/sideEffects.test.ts | New test verifying that side effects don't execute when a module is mocked. |
| e2e/mock/src/sideEffects.ts | Test fixture module with side effects. |
| e2e/mock/fixtures/unmock/unmock.test.ts | Enhanced test to verify unmock functionality with custom modules. |
| e2e/mock/fixtures/unmock/rstest.setup.ts | Added mock setup for custom module to test unmock behavior. |
| e2e/mock/fixtures/unmock/noUnmock.test.ts | Test to verify mocked state persists without unmock. |
| e2e/mock/fixtures/unmock/a.ts | Simple test fixture module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
👍🏻 |
Summary
should not load real implementation when module is mocked.
rs.mock()to check the module cache first and avoid loading modules unless necessary (e.g., for spy/mock options)Related Links
fix: #745
Checklist