Walkthrough

Added clearMocks: true to the base Jest configuration and introduced a test that verifies mock call histories are cleared between tests while mock implementations persist.

Changes

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I bound through tests with nimble paws,
Clearing footprints without a pause,
One test calls loud, the next is new,
No traces left — a meadow true,
Hooray for tidy mock applause!

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Greptile Summary

This PR adds clearMocks: true to the shared jest.config.base.js, ensuring that mock call history (mock.calls, mock.instances, mock.results, mock.contexts) is automatically reset between tests for all packages that inherit this config (react-on-rails and react-on-rails-pro). The create-react-on-rails-app package maintains its own standalone config and is unaffected.

• clearMocks: true added to jest.config.base.js with a comment
• The comment slightly overstates the effect by saying "implementations" are cleared — clearMocks only wipes call history, not mock implementations (that requires resetMocks: true; create-react-on-rails-app already uses both)
• The functional change itself is correct and harmless — no tests relied on mock call history persisting across tests

Confidence Score: 5/5

• This PR is safe to merge — it adds a single, well-understood Jest option to the shared base config with no risk of breaking existing tests.
• The change is a one-liner that enables automatic mock call history clearing, a standard Jest best practice. No tests depend on mock state persisting between tests (which would be a bug), and the option is already in use in the create-react-on-rails-app package. The only minor issue is a misleading comment.
• No files require special attention.

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Base["jest.config.base.js\n(clearMocks: true ✅ NEW)"]
    ROR["packages/react-on-rails\njest.config.js\n(...rootConfig)"]
    RORPRO["packages/react-on-rails-pro\njest.config.js\n(...rootConfig)"]
    CRORA["packages/create-react-on-rails-app\njest.config.js\n(standalone — not affected)"]

    Base -->|inherits| ROR
    Base -->|inherits| RORPRO
    CRORA -.->|already has clearMocks + resetMocks| CRORA

_{Last reviewed commit: ffe094c}

Good, low-risk improvement for test isolation. One thing to flag.

Inaccurate comment / PR description

The inline comment (line 42) and the PR description both say clearMocks clears "implementations", but that is not what it does:

• clearMocks: true clears call history only (calls, instances, results). Does NOT reset implementations.
• resetMocks: true clears call history AND resets implementations to jest.fn() with no implementation.
• restoreMocks: true clears call history AND restores original (non-mocked) implementation.

clearMocks only zeros out .mock.calls, .mock.instances, .mock.contexts, and .mock.results before each test. Implementations set via .mockImplementation() or jest.mock() are untouched. I have left a suggestion on the comment line.

Is clearMocks the right level here?

Yes -- resetMocks would be too aggressive for this codebase. Both jest.setup.js files assign global.MessageChannel and global.console as jest.fn() mocks via setupFiles (which runs once per test file, not before each individual test). Resetting implementations with resetMocks would silently break those globals for every test after the first. clearMocks is the correct choice.

Consistency note

packages/create-react-on-rails-app/jest.config.js does not inherit from jest.config.base.js, so it is unaffected by this PR. It already has both clearMocks: true and resetMocks: true set independently -- no action needed there.

Summary

The change itself is correct and beneficial. The only fix needed is the comment text so it accurately reflects what clearMocks actually does.

Review: Enable clearMocks in shared Jest config

The change is small, focused, and well-justified. A few observations:

What's correct:

• clearMocks: true resets .mock.calls, .mock.instances, .mock.results, and .mock.contexts between tests without touching mock implementations. The inline comment accurately describes this distinction.
• All test files in the affected packages (react-on-rails, react-on-rails-pro) set up their mock implementations in beforeEach or via jest.mock() module factories that survive clearMocks, so no existing tests should break.
• Tests that compare mock.calls.length (e.g. ClientRenderer.test.ts) do so with relative baselines captured within a single test, which is unaffected by inter-test clearing.

Worth noting:

• packages/create-react-on-rails-app/jest.config.js uses its own standalone config (not inheriting from jest.config.base.js) and already enables both clearMocks: true and resetMocks: true. If test isolation issues arise for the packages inheriting the base config, resetMocks: true could be considered as a follow-up — but clearMocks alone is the safer starting point since it preserves implementations set outside beforeEach.

Overall this is a clean, low-risk improvement to test hygiene. LGTM.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Overall: LGTM with one minor suggestion. The change is clean, well-scoped, and low-risk. Adding clearMocks: true to the shared base config is the right conservative choice — it clears call history between tests without resetting mock implementations, so existing tests relying on persistent mockReturnValue or mockImplementation setups should not break. The comment accurately distinguishes clearMocks from resetMocks (a common source of confusion), and adding a regression test is a nice touch. One minor gap: the test only verifies that call history is cleared, but does not confirm that mock implementations are preserved — the key semantic difference between clearMocks and resetMocks. A third test asserting that a mockReturnValue set in one test still works in the next would make this a more complete spec. Not blocking — see inline comment for the specific suggestion.

size-limit report 📦

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Overall: Good, focused change. Enabling clearMocks:true in the shared config prevents call-count and instance leakage between tests without risking breaks for tests that rely on mock return values.

Concern: clearMocks vs resetMocks

packages/create-react-on-rails-app/jest.config.js already ships with both clearMocks:true and resetMocks:true. resetMocks does everything clearMocks does, PLUS it resets mock implementations back to jest.fn() after each test.

The PR is intentionally conservative by stopping at clearMocks, but the consequence is that mock implementations (mockReturnValue, mockResolvedValue, etc.) can still bleed between tests - exactly what the third test in clearMocksConfig.test.ts demonstrates. If issue 2500 was caused by stale implementations rather than stale call counts, this fix may not be sufficient.

If the existing test suites pass with resetMocks:true added to the base config too, that would provide significantly stronger isolation and align with what create-react-on-rails-app already uses.

Minor nit on the test

The third test ("preserves mock implementations across tests") intentionally relies on mockReturnValue("first") being set by the first test. That order dependency is by design for this meta-test, but a short inline comment would help future readers understand this demonstrates a known limitation of clearMocks - in real test suites, implementation bleed is a footgun, not a feature.

Nothing blocking - the change is safe, well-documented, and the test clearly verifies the intended behaviour.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Config change (jest.config.base.js): Adding clearMocks: true to the shared base config is a solid improvement. The inline comment correctly documents the distinction from resetMocks. This belongs in the shared base so all packages inherit it automatically.

Test file (clearMocksConfig.test.ts): Two issues worth addressing.

1. Test 3 will break under randomised execution order. Jest 28+ supports --randomize which shuffles test order within a file. If test 3 runs before test 1, sharedMock() returns undefined (not "first"), causing the assertion to fail. The comment acknowledges the dependency but that does not make it safe.

2. It normalises an antipattern. Future contributors reading this file will see the module-level jest.fn() and the "Intentional cross-test dependency" label and may treat this as a precedent for their own tests.

Suggestion: If keeping the test file, rewrite test 3 to set up its own implementation (a local mockReturnValue call or a beforeEach) so all three tests pass regardless of execution order. Alternatively, drop the test file and rely on the config comment plus Jest documentation.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Overall: Clean, low-risk improvement. Enabling clearMocks: true is a well-established Jest best practice for test isolation and the implementation is correct.

What is good

• Correct semantics: clearMocks clears call history/instances/results between tests without resetting mock implementations. The inline comment distinguishing it from resetMocks is helpful.
• Propagation is correct: Both packages/react-on-rails and packages/react-on-rails-pro spread the base config and inherit the setting automatically. create-react-on-rails-app has its own standalone config (clearMocks plus resetMocks) and is unaffected.
• Test coverage: The test file correctly validates all three key behaviors -- call-history clearing, implementation persistence, and count reset.

Minor observations

1. beforeAll is the right choice (not beforeEach) since clearMocks leaves implementations intact. If this were ever changed to resetMocks: true, the test would start failing -- which is the desired signal. Intentional and correct.

2. Test name is slightly ambiguous (line 14): 'clears call history before the next test' describes a future event rather than asserting what happened before this test ran. A name like 'has cleared call history from the previous test' would be more precise -- cosmetic only.

3. Test file placement: The file validates base config behavior that affects all packages but lives in packages/react-on-rails/tests/. Pragmatic, but coverage of the base config is coupled to one package's test run. Not a blocker.

No blocking issues. Safe to merge.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Overall: Clean, well-scoped change.

Adding clearMocks: true to jest.config.base.js is the right call for improving test isolation. This is consistent with create-react-on-rails-app/jest.config.js, which already independently sets clearMocks: true (and resetMocks: true).

Positive aspects:

• Comment in the config clearly differentiates clearMocks vs resetMocks behavior.
• New test file correctly validates the exact semantics: call history cleared between tests, implementations persist.
• Existing ClientRenderer.test.ts tests are unaffected: they compare call counts within a single it block, so between-test clearing does not change their semantics.

Observations:

1. clearMocksConfig.test.ts tests are intentionally order-dependent. Acceptable for a config-verification test. If --randomize is ever adopted monorepo-wide, this file would need an explicit ordering annotation. Not a blocker.

2. Meta-test pattern is unusual but valid. Testing Jest config behavior serves as a regression guard: if clearMocks: true is accidentally removed, these tests fail immediately rather than letting hidden mock-state bugs silently creep back in.

3. create-react-on-rails-app is unaffected -- its jest.config.js is a standalone CJS config that does not extend the base.

No security or performance implications. No issues found with existing tests.

Review

The change is clean and well-scoped. A few observations:

jest.config.base.js
The comment accurately distinguishes clearMocks from resetMocks. One thing worth noting for future contributors: clearMocks: true clears call history/instances/results between tests but does NOT reset mock implementations. This means if a test sets mockReturnValue (or similar) inside the test body rather than in a setup hook, that implementation will persist into subsequent tests. This is the intentional, less-destructive choice vs resetMocks, but worth keeping in mind when writing new tests.

Scope of impact
create-react-on-rails-app has its own standalone config (uses module.exports, does not spread the base config) with both clearMocks: true AND resetMocks: true already, so this PR does not change behavior there. The two packages that inherit from the base (react-on-rails and react-on-rails-pro) will get the new behavior.

Test file (clearMocksConfig.test.ts)
The test correctly validates the semantics and serves as living documentation. The three tests are intentionally order-dependent (test 2 asserts that test 1's calls were cleared), which is the right approach for testing this specific behavior. The leading comment covers the 'what' but could also note that the ordering of tests within the suite is significant - makes the intent clearer to future readers.

Overall this is a good, low-risk improvement to test isolation. LGTM.

Code Review

This is a clean, well-scoped PR that improves test isolation across the monorepo. Adds clearMocks: true to the shared jest.config.base.js (resetting mock call history between tests without touching implementations) and a focused regression test to verify the semantics.

Config change: Minimal, well-commented, and correctly distinguishes clearMocks from resetMocks. Standard Jest best practice for a shared monorepo config.

Test file: The three-test sequence correctly validates: (1) calls are recorded, (2) call history is cleared between tests, (3) implementations persist. Module-level comment explaining the intentional shared state is a nice touch.

Potential downstream impact: clearMocks: true could surface previously hidden cross-test mock state dependencies in packages inheriting this config, which the PR description correctly frames as the intended benefit.

Overall a straightforward, low-risk improvement. LGTM.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Overall: A clean, low-risk improvement to test isolation. The config change is correct and the new test file effectively validates the behavior.

jest.config.base.js

The addition of clearMocks: true is sound:

• Does not break setupFiles mocks. jest.setup.js creates global.MessageChannel, global.console, etc. as jest.fn(). With clearMocks: true, call histories are wiped between tests but mockImplementations survive, so MessageChannel still functions correctly.
• create-react-on-rails-app is unaffected — its jest.config.js is standalone CommonJS (does not spread rootConfig) and already has both clearMocks: true and resetMocks: true.
• Not adding restoreMocks: true is correct. Tests in StoreRegistry.test.js and ComponentRegistry.test.js already call consoleSpy.mockRestore() manually — adding restoreMocks to the base config would be redundant and could cause unexpected behavior where console is already a jest.fn().

clearMocksConfig.test.ts

Two inline comments posted — a robustness note about the intentional order-dependency and a suggestion to document the beforeAll vs beforeEach choice. Neither is a blocker.

Review: Enable clearMocks in shared Jest config

Overall: Looks good — clean, well-scoped change. Two minor comments below.

jest.config.base.js

Adding clearMocks: true to the base config is the right call for a monorepo. It prevents mock call-history leaking between tests without touching mock implementations, which is the least-surprising default for most test suites. The comment accurately explains the distinction vs resetMocks.

One suggestion: the comment could also mention restoreMocks: true so readers know the full ladder (clearMocks → resetMocks → restoreMocks) — see inline.

clearMocksConfig.test.ts

The three-test structure effectively documents what clearMocks does and doesn't do. One minor documentation issue: the header comment says the suite is "order-dependent by design," which conflicts with an earlier commit message ("Make clearMocks regression test order-independent") and is subtly inaccurate — the assertions hold in any Jest-execution order within the describe block. The real caveat is narrower: the not.toHaveBeenCalled() assertion only proves clearMocks fired when at least one prior test called the mock. See inline for a suggested rewording.

No correctness issues; the configuration change and its test coverage are solid.