Skip to content

interopDefault's caching Proxy (#421) breaks ESM live bindings for mutable exports #457

Description

@Margesh06

Environment

jiti: 2.7.0
node: v20.20.1
os: macOS (darwin)

Reproduction

No separate repo — this reproduces in 3 files and ~15 lines, deterministic,
no environment-specific behavior. Setup:

```bash
mkdir repro && cd repro
npm init -y
npm install jiti@2.7.0
```

Then add:

```js
// counter.mjs
export let counter = 1;
export function increment() {
  counter++;
}
```

```js
// repro.cjs
const createJiti = require("jiti");
const jiti = createJiti(__filename, {
  interopDefault: true,
  fsCache: false,
  moduleCache: false,
});
const mod = jiti("./counter.mjs");
console.log("first read:", mod.counter); // 1
mod.increment();
console.log("after increment():", mod.counter); // still 1 -- should be 2
```

```
$ node repro.cjs
first read: 1
after increment(): 1
```

Reproduced live against `jiti@2.7.0` (current published version as of this
writing) and independently against a fresh clone/build of `unjs/jiti` at
`fd3bb289b75ed207edfb686d671ed50144f7e90f` — same result both times.

### Describe the bug

```markdown
`jitiInteropDefault`'s Proxy caches every property read in a `Map` and never
invalidates it. That means the *first* read of any property permanently locks
in that value — including for properties backed by a getter re-exporting a
mutable ESM binding (`export let x`, incremented counters, mutable config
singletons, feature flags, etc.). Since `interopDefault` defaults to `true`,
this silently affects every module jiti loads by default.

Expected: after calling `increment()`, a subsequent read of `mod.counter`
should return `2`, matching how every other ESM→CJS interop path (and native
ESM) treats a mutable export — each read re-observes the current value, since
jiti's own Babel CJS transform compiles a mutable export into a live getter on
`exports` specifically to preserve this semantics.

Actual: the read stays frozen at `1` forever after the first access, with no
error or warning.

I have a regression test and a candidate fix ready and would like to open a PR
against this issue if that's a welcome direction — see "Additional context"
for the fix approach and root cause; happy to go with whichever the
maintainers prefer.

### Additional context

```markdown
## Root cause

This regressed in `a467d31` ("perf(interopDefault): add caching to reduce
proxy overhead by ~2x", #421). Before that commit, every property read did a
fresh `Reflect.get`, correctly re-invoking the getter each time and preserving
live-binding semantics. The caching Proxy added in #421 wraps every module
jiti loads and memoizes the result of the first `Reflect.get` per property in
a `Map`, never invalidating it — so any property backed by a getter (rather
than a static value), which is exactly how the Babel CJS transform represents
a mutable ESM export, gets its first-observed value frozen forever.

## Why it matters

Any real code relying on `.import()`/`jiti()` interop for a module with a
mutable export — config singletons, ID counters, feature flags reassigned at
runtime — silently sees a stale value with no error or warning. This only
shows up for *mutable* exports, so anyone who tested only with `const` exports
(the common case) won't hit it, making it easy to ship and hard to notice
until it corrupts state in production.

## Suggested fix

Either drop the cache entirely and always call `Reflect.get`, or only memoize
properties that are plain data properties (check the descriptor via
`Object.getOwnPropertyDescriptor` and skip caching when `desc.get`/`desc.set`
is present). The second option preserves most of #421's intended perf win for
ordinary CJS `module.exports` while not breaking getter-backed mutable
exports.

## Regression test

A `describe("jitiInteropDefault")` block added to `test/utils.test.ts` wraps a
mock module shaped exactly like jiti's real Babel-CJS output (a live getter +
a mutator function) with the real exported `jitiInteropDefault`, and asserts
the read value updates after mutation. Fails on current `main`
(`expected 1 to be 2`); passes if the pre-`a467d31` implementation is swapped
in temporarily — confirming the test correctly isolates this regression.
```

### Logs

```sh
$ node repro.cjs
first read: 1
after increment(): 1

# expected:
first read: 1
after increment(): 2
```

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions