fix(core): share imported module state and keep context-bound APIs live across files under isolate: false#1376
Draft
fi3ework wants to merge 1 commit into
Draft
fix(core): share imported module state and keep context-bound APIs live across files under isolate: false#1376fi3ework wants to merge 1 commit into
fi3ework wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ad9c97bfa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Rsdoctor Bundle Diff AnalysisFound 13 projects in monorepo, 2 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 core/browserPath:
📦 Download Diff Report: core/browser Bundle Diff 📁 core/mainPath:
📦 Download Diff Report: core/main Bundle Diff Generated by Rsdoctor GitHub Action |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e703cf8d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
pkg.pr.new preview
|
fi3ework
added a commit
that referenced
this pull request
Jun 4, 2026
Under `isolate: false` the shared-module fix persists user modules across files, but `test.extend(...)` captured the per-file runner eagerly: a fixture module doing `export const test = base.extend(...)` evaluated once, so from the second file on every registration hit the first file's torn-down runner and threw "Test '<name>' cannot run". Make the runner API intrinsically late-bound, mirroring the existing `RSTEST_API` proxy: publish the current file's runner as `globalThis.RSTEST_RUNTIME` (reassigned per file) and resolve it at every leaf registration call instead of closing over `runtimeInstance`. `getLocation` moves onto the runner so a persisted extended test also computes locations against the current file. Closes #1376.
pkg.pr.new preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 101313be9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fi3ework
added a commit
that referenced
this pull request
Jun 4, 2026
A follow-up to the test.extend() fix: under `isolate: false` a context-bound
`@rstest/core` API value-copied into a module shared across files goes stale.
A shared helper doing `Object.assign(impl, expect)` (or `const { poll } = expect`)
captures the first file's concrete `expect`, whose `expect.poll` is bound to that
file's `getCurrentTest`; from the second file on it threw "expect.poll() must be
called inside a test".
Make the per-file `expect` self-delegate: when invoked from a stale cross-file
reference it forwards to the current file's live expect (`globalThis[GLOBAL_EXPECT]`,
reassigned per file), so test attribution and assertion state track the running
file. The per-test local expect leaves `isFileExpect: false` so it never delegates
and keeps `test.concurrent` assertion isolation. For the current file's own expect
`live === expect`, so this is a no-op fast path.
Refs #1376.
pkg.pr.new preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a4cc28506
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fi3ework
added a commit
that referenced
this pull request
Jun 4, 2026
…solate: false Continues the #1376 family: under `isolate: false` a context-bound `@rstest/core` API value-copied into a module shared across files goes stale. The file-level hooks (`afterAll`/`beforeAll`/`afterEach`/`beforeEach`) were left bound to the first file's runner, so a shared helper re-exporting a hook (e.g. `export const beforeEach = beforeEach`) silently registered into the first file's torn-down collector from the second file on. `onTestFinished`/ `onTestFailed` had the same staleness against the per-file test runner and threw "can only be called inside a test". Route the hooks through `currentRuntime()` (the per-file `RSTEST_RUNTIME`, like `it`/`describe`), and `onTestFinished`/`onTestFailed` through a new per-file `globalThis.RSTEST_RUNNER` resolved at call time. The remaining `rstest`/`rs` utilities object is the one known, non-idiomatic residual (re-exporting the whole namespace), deferred until a real repro surfaces. Refs #1376.
pkg.pr.new preview
|
Deploying rstest with
|
| Latest commit: |
88fac87
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ee874416.rstest.pages.dev |
| Branch Preview URL: | https://fix-no-isolate-module-sharin.rstest.pages.dev |
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 429ac70f7f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
fi3ework
added a commit
that referenced
this pull request
Jun 5, 2026
Under `isolate: false` a single worker runs many test files: `@rstest/core` is reset per file (its module cache is evicted and re-imported, #1373) while user modules persist — a helper imported by several files is evaluated only once. Two problems followed from that asymmetry: 1. Context-bound `@rstest/core` APIs value-copied into a persisted helper (e.g. `export const test = base.extend({})`, a re-exported hook, `onTestFinished`, `{ ...rstest }`, or a snapshotted `expect.poll`) froze to the FIRST file's torn-down context and silently misbehaved from the second file on. 2. Setup files were matched against the module cache by raw path, so on Windows / mixed separators the cache-control plugin failed to re-run them per file. Fix the binding problem with a single live-binding contract: each context-bound member is a STABLE forwarder, built once at module load, that re-resolves the running file's state from a per-file global slot at call time — never closing over a per-file instance. The collection surface (`runtimeAPI`) and the full runner surface (`runnerAPI`) are built once; `createRuntimeAPI` only constructs the file's runtime and publishes its slot. `expect` additionally self-delegates to the live `GLOBAL_EXPECT` to rescue an already-copied `expect.poll`/`.soft`. The contract (the four slots, the shared value-copy residual, and why `expect` is the one self-delegating member) is documented in one place, `runtime/api/index.ts`. Fix the setup-file problem by normalizing paths with `pathe` before matching. Add an e2e surface guard that drives the whole context-bound API surface (`test.extend`, hooks, `describe`, `expect.poll`, `rstest.fn`/`clearMocks`) through a persisted shared helper from a non-first file, and document that module-scope code runs once per worker under `isolate: false`. Closes #1376
fi3ework
force-pushed
the
fix-no-isolate-module-sharing
branch
from
429ac70 to
a8d3b0f
Compare
fi3ework
changed the title
fi3ework
force-pushed
the
fix-no-isolate-module-sharing
branch
7 times, most recently
from
bdf1fe3 to
4f70360
Compare
Under `isolate: false` a single worker runs many test files: `@rstest/core` is reset per file (its module cache is evicted and re-imported, #1373) while user modules persist — a helper imported by several files is evaluated only once. Two problems followed from that asymmetry: 1. Context-bound `@rstest/core` APIs value-copied into a persisted helper (e.g. `export const test = base.extend({})`, a re-exported hook, `onTestFinished`, `{ ...rstest }`, or a snapshotted `expect.poll`) froze to the FIRST file's torn-down context and silently misbehaved from the second file on. 2. Setup files were matched against the module cache by raw path, so on Windows / mixed separators the cache-control plugin failed to re-run them per file. Fix the binding problem with a single live-binding contract: the whole injected API surface (`runtimeAPI`, `runnerAPI`, `expect`, `rstest`) is built ONCE per worker with a stable identity, and every context-bound member resolves the running file's `FileContext` at call time — one module-level binding holding the file's worker state, collection registrar, and test runner, republished as one unit by `createRunner` — never closing over a per-file instance. Per-file state is RESET, not rebuilt: `expect` clears its assertion bookkeeping and re-establishes the live `testPath` getter, `rstest` drops its stub/timer/mock bookkeeping and rewinds the shared `invocationCallOrder` counter. Because the file-level `expect` is a build-once singleton, a value-copied `expect.poll`/`.soft` can never go stale, so no cross-file delegation is needed; the per-test local expect intentionally stays a pinned per-test instance to keep `test.concurrent` isolation. Construction and publication are separated (`createRuntimeAPI` is a pure factory; only `createRunner` publishes the context), and globalThis carries only true cross-boundary contracts (`RSTEST_API` for the user bundle, `GLOBAL_EXPECT` for the assertion ecosystem). The contract is documented in one place, `runtime/api/index.ts`. Fix the setup-file problem by normalizing paths with `pathe` before matching. Sharing user modules means keeping the runtime chunk (which owns the only `__webpack_module_cache__`) across files. In watch mode the pool reuses the worker across rebuilds, so that kept cache also survived rebuild boundaries and a changed shared module was served stale from the previous build. Thread a per-compile `buildId` to the worker and fully flush its module cache when the id changes, so every rebuild re-evaluates shared modules while in-build sharing is preserved. Add an e2e surface guard that drives the whole context-bound API surface (`test.extend`, hooks, `describe`, `expect.poll`, `rstest.fn`/`clearMocks`) through a persisted shared helper from a non-first file, a watch regression test asserting an edited shared module is re-evaluated on rerun, and document that module-scope code runs once per worker under `isolate: false`. Closes #1373
fi3ework
force-pushed
the
fix-no-isolate-module-sharing
branch
from
4f70360 to
88fac87
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88fac87241
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // unwound elsewhere (the runner's config-gated `unstubAll*`/`*AllMocks` and the | ||
| // per-file `useRealTimers`), so only the tracking maps/registry are cleared. | ||
| const resetForFile = (): void => { | ||
| mocks.clear(); |
There was a problem hiding this comment.
When isolate: false shares a helper that creates export const mock = rstest.fn() while file A is collected, file B imports the cached helper rather than creating the mock again. This reset removes that still-live mock from the only mocks registry, so file B's configured clearMocks/resetMocks/restoreMocks hooks and manual rstest.clearAllMocks() no longer touch it, letting call history or implementations leak between tests despite the reset options.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Under
isolate: false, one worker process runs many test files:@rstest/coreis reset per file (its module cache is evicted and re-imported) while user modules persist — a helper imported by several files is evaluated only once per worker. Two problems followed from that asymmetry, both fixed here.1. Imported module state was not shared across files (#1373)
The worker teardown cleared the entire module cache after every file, evicting the shared per-worker runtime chunk that owns the only
__webpack_module_cache__. The next file re-instantiated that runtime, so every bundled module factory ran again and module-scope state (singletons, caches, lazily-initialized resources) did not survive across files — defeating the main reason to opt into non-isolated mode.Teardown invalidation is now selective:
__webpack_module_cache__— and the state of every already-evaluated non-entry module — persists across files. Only the test-entry and setup modules are evicted so their bodies re-run per file.import.metahooks (wasm / dynamic-import resolution). The map is reset only on a full cache clear.pathebefore matching, so setup files reliably re-run per file on Windows / mixed separators (previously a raw-path collection clobber left the targeted setup-reset list empty).2. Context-bound APIs went stale when value-copied into a shared helper
Because user modules persist, any context-bound
@rstest/corevalue captured at module top-level in a persisted helper —export const test = base.extend({}), a re-exportedonTestFinished,{ ...rstest }, or a snapshottedexpect.poll— froze to the first file's torn-down context and silently misbehaved from the second file on.Fixed with a single live-binding contract: each context-bound member is a stable forwarder, built once, that re-resolves the running file's state from a per-file global slot at call time — never closing over a per-file instance. The collection surface (
runtimeAPI) and the full runner surface (runnerAPI) are built once;createRuntimeAPIonly constructs the file's runtime and publishes its slot.expectadditionally self-delegates to the liveGLOBAL_EXPECTto rescue an already-copiedexpect.poll/.soft. The contract — the four slots, the shared value-copy residual, and whyexpectis the one self-delegating member — is documented in one place (runtime/api/index.ts).3. Shared module state went stale across watch rebuilds
Sharing user modules means keeping the runtime chunk (which owns the only
__webpack_module_cache__) across files. In watch mode the worker pool is reused across rebuilds, so that kept cache also survived rebuild boundaries — underisolate: falsea changed shared module was re-run with the previous build's exports until the process restarted.A per-compile
buildIdis now threaded to the worker. When it changes (every watch rebuild), the worker fully flushes its module cache before loading the new build, so each rebuild re-evaluates shared modules. Intra-build sharing across files is unchanged.Behavior
With
isolate: false, an imported module now evaluates once per worker and is shared across the files that run in it, and context-bound APIs always act on the running file; the test entry, setup files, and@rstest/corestill reset per file. Applies to both ESM and CommonJS output.A new e2e surface guard drives the whole context-bound API surface (
test.extend, hooks,describe,expect.poll,rstest.fn/clearMocks) through a persisted shared helper from a non-first file; a watch regression test asserts that an edited shared module is re-evaluated on rerun; and theisolatedocs note that module-scope code runs once per worker (use setup files for per-file setup).Related Links
isolate: false#1373Checklist