diff --git a/.changeset/fix-hmr-fast-refresh-minified-name.md b/.changeset/fix-hmr-fast-refresh-minified-name.md new file mode 100644 index 0000000..997da38 --- /dev/null +++ b/.changeset/fix-hmr-fast-refresh-minified-name.md @@ -0,0 +1,7 @@ +--- +"react-call": patch +--- + +Fix: Vite/React Fast Refresh no longer rejects a Callable as "incompatible" in the published build, so editing a Callable's own source while a call is open hot-updates in place instead of triggering a full reload that closes it ([ADR-0022](docs/adr/0022-pin-callable-name-for-fast-refresh.md), [#110](https://github.com/desko27/react-call/issues/110)). + +The minified bundle renamed the returned component's function to a lowercase identifier, which fails react-refresh's `isLikelyComponentType` name check — and because the truthy minified `name` shadows `displayName`, neither setting `displayName` manually nor the `react-call/vite` plugin could work around it. `createCallable` now pins an uppercase `name` on the Callable (dev-only; production output is byte-for-byte unchanged), restoring Fast Refresh acceptance for the artifact consumers actually install. diff --git a/docs/adr/0009-hmr-and-fast-refresh.md b/docs/adr/0009-hmr-and-fast-refresh.md index e3fd159..5012541 100644 --- a/docs/adr/0009-hmr-and-fast-refresh.md +++ b/docs/adr/0009-hmr-and-fast-refresh.md @@ -1,6 +1,6 @@ # Vite HMR + Fast Refresh: Callable as a component, store in a registry -> **Partially superseded by [ADR-0010](./0010-callable-displayname-as-hmr-key.md).** The registry mechanism, the `Object.assign(Root, {...})` shape, and the upsert-state move to the store all stand. The registry **key derivation** (`UserComponent.displayName ?? UserComponent.name`) and the rejection of the explicit-ID option are reopened and resolved in ADR-0010. +> **Partially superseded by [ADR-0010](./0010-callable-displayname-as-hmr-key.md) and [ADR-0022](./0022-pin-callable-name-for-fast-refresh.md).** The registry mechanism, the `Object.assign(Root, {...})` shape, and the upsert-state move to the store all stand. The registry **key derivation** (`UserComponent.displayName ?? UserComponent.name`) and the rejection of the explicit-ID option are reopened and resolved in ADR-0010. The claim below that making the return value a function flips Fast Refresh to **compatible** holds only against source: the published bundle minifies the `Root` name to a lowercase identifier, so `isLikelyComponentType` rejects it until ADR-0022 pins an uppercase `.name` (issue #110). The object `createCallable()` returns is the `Root` component itself with the imperative methods (`call`, `upsert`, `end`, `update`, plus a `Root` self-reference for backwards compatibility) attached as properties via `Object.assign`. The store the Callable closes over no longer lives only in the createCallable closure — it is also registered in a module-level `Map` inside `react-call`'s own (HMR-immune) module, keyed by `UserComponent.displayName ?? UserComponent.name`. Together these two changes close GitHub issue #31: `vite-plugin-react`'s Fast Refresh accepts `Confirm.tsx` (no more "Could not Fast Refresh" warning, no full page reload), and for any UserComponent that has a name the open dialog survives across saves of its own source file. diff --git a/docs/adr/0022-pin-callable-name-for-fast-refresh.md b/docs/adr/0022-pin-callable-name-for-fast-refresh.md new file mode 100644 index 0000000..6b1ba16 --- /dev/null +++ b/docs/adr/0022-pin-callable-name-for-fast-refresh.md @@ -0,0 +1,24 @@ +# Pin an uppercase `Callable.name` so Fast Refresh accepts the published bundle + +`createCallable` assigns the returned Callable a stable uppercase name — `Object.defineProperty(callable, 'name', { value: 'Root', configurable: true })` — inside the same dev-only `process.env.NODE_ENV !== 'production'` block that owns the `displayName` setter (ADR-0010/0011). React Fast Refresh decides whether a module export is a component through react-refresh's `isLikelyComponentType`, which for a function gates on `/^[A-Z]/.test(fn.name || fn.displayName)`. ADR-0009 made the Callable a *function* (the `Root` component via `Object.assign`) precisely to clear that bar — but the bar is on the *runtime name*, and the library's own build minifies `function Root` down to a lowercase identifier (`p` in the current bundle). So the export react-refresh sees in a consumer's app has `name === 'p'`, fails the regex, and the boundary is rejected: `Could not Fast Refresh ("Confirm" export is incompatible)`, a full reload, and the open dialog vanishes (issue #110). + +This also explains why `displayName` — ADR-0010's HMR key, ADR-0012's auto-injected line — never rescued it: `fn.name || fn.displayName` short-circuits on the truthy minified `fn.name` before `displayName` is ever read. And it explains why the regression survived CI for a whole release line: every playground aliases `react-call` to source (ADR-0005), where `function Root` keeps its uppercase name and Fast Refresh accepts, so the bug exists only in the minified artifact consumers install. Pinning `.name` to `'Root'` at runtime restores the uppercase name the minifier strips, so the published bundle clears the bar the source always did. + +The reframing for ADR-0009: `Object.assign` making the return value a function is *necessary but not sufficient* for the artifact. Two things are now load-bearing for Fast Refresh acceptance — the function shape (ADR-0009) **and** the pinned uppercase name (this ADR) — and both are independent of the `displayName`/registry layer (ADR-0010/0011/0012), which is solely about keeping an open call's state alive across a save. + +## Considered options + +- **Pin `.name` at runtime via `defineProperty`, dev-only** (chosen). Deterministic, minifier-proof, and free in production: the assignment lives in the `NODE_ENV !== 'production'` block, so the consumer's prod build dead-code-eliminates it exactly like the `displayName` setter (ADR-0011). `function.name` is `configurable: true`, so redefining it is legal and the value survives every later bundler pass. +- **Enable `keepNames` (or disable identifier minification) in the library build** so `function Root` keeps its name. Rejected: `keepNames` preserves *every* identifier and injects `__name` helper calls, inflating the bundle for the one name that matters — a poor trade against the 1 KB `size-limit` budget (ADR-0007) — and it couples the runtime contract to a build-config flag instead of stating it in code. +- **Have the `displayName` setter also write `.name`.** Rejected: it only helps once a `displayName` is assigned, which reintroduces the "you must name it" requirement ADR-0009 explicitly wanted gone for the anonymous-arrow case. Fast Refresh acceptance should be unconditional; state persistence is the opt-in. +- **Derive `.name` from the consumer's variable or `displayName`.** Rejected as needless: `isLikelyComponentType` only inspects the first character, and the registry already keys on `displayName`, so a constant `'Root'` is sufficient and cannot collide. `'Root'` also matches the source function's name and the `Root` vocabulary in CONTEXT.md. +- **Pin in both dev and prod.** Rejected: production never runs Fast Refresh, so the name earns nothing there; gating keeps the prod bundle byte-identical to a build without this feature, per ADR-0011. + +## Consequences + +- **Fast Refresh acceptance now holds for the published artifact, not only source.** ADR-0009's "making the return value a function flips the export from incompatible to compatible" is corrected: it flips it in source; the pinned name is what carries that across minification. ADR-0009 carries a header note pointing here. +- **Every Callable reports `name === 'Root'` in dev.** React DevTools shows the `displayName` when one is set (manually or via `react-call/vite`) and falls back to `'Root'` otherwise — identical to how a source build already behaves. The uniform name does **not** collide in the registry: adoption keys on the `displayName` string (ADR-0010), never on `.name`. +- **`displayName` and the `react-call/vite` plugin are now exclusively about HMR state persistence**, fully decoupled from Fast Refresh acceptance. A Callable with no `displayName` hot-updates in place (no warning, no reload) but its own open call still resets on save; adding `displayName` is what keeps it open. +- **A guard test lives in the `dist` vitest project, not `unit`.** `packages/react-call/src/fast-refresh-compat.test.ts` imports the built `dist/main.js` and asserts a verbatim copy of `isLikelyComponentType` accepts the Callable. A source-level test cannot reproduce the failure — source keeps the `Root` name — which is the same blind spot that let issue #31's fix (ADR-0009) pass CI yet regress for consumers. The test is registered in `DIST_TESTS` in `vitest.config.ts` alongside `bundle-content` and `tarball-snapshot`, so it runs against `pnpm build` output under `test:dist`. +- **The production bundle is unchanged.** The pin is dev-gated; `dist/main.js` stays at 810 B brotli, within the 1 KB `size-limit` budget (ADR-0007). +- **`CONTEXT.md` is unchanged.** `Root` already names the mounting form of a Callable; pinning the runtime `.name` to `'Root'` reuses that vocabulary rather than introducing a new term. diff --git a/packages/react-call/src/createCallable/index.tsx b/packages/react-call/src/createCallable/index.tsx index 3b2057e..d60c4fd 100644 --- a/packages/react-call/src/createCallable/index.tsx +++ b/packages/react-call/src/createCallable/index.tsx @@ -172,6 +172,11 @@ export function createCallable( // the consumer's module); this whole block is dead code in the // consumer's production bundle. if (process.env.NODE_ENV !== 'production') { + Object.defineProperty(callable, 'name', { + value: 'Root', + configurable: true, + }) + let displayName: string | undefined let registered = false Object.defineProperty(callable, 'displayName', { diff --git a/packages/react-call/src/fast-refresh-compat.test.ts b/packages/react-call/src/fast-refresh-compat.test.ts new file mode 100644 index 0000000..2593af9 --- /dev/null +++ b/packages/react-call/src/fast-refresh-compat.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' + +// Load the BUILT dist on purpose: this property only holds for the +// published, minified artifact, never the source. The repo's playgrounds +// alias react-call to src (ADR-0005), where the component function keeps +// its uppercase `Root` name, so a source-level test cannot catch the +// regression this guards (ADR-0022, issue #110). +// +// The dist only exists under `test:dist` (after `pnpm build`), not during +// `tsc` type-check, so we import it through a runtime specifier TS won't +// statically resolve (a non-literal `string`), typed against the source +// barrel. This file stays type-checked without depending on the build. +const DIST_ENTRY: string = '../dist/main.js' +const { createCallable }: typeof import('./main') = await import(DIST_ENTRY) + +// React Fast Refresh decides whether a module export is a component via +// react-refresh's `isLikelyComponentType`. For functions it gates on +// `/^[A-Z]/.test(type.name || type.displayName)`. Verbatim copy of the +// `function` branch from @vitejs/plugin-react's refresh-runtime.js +// (byte-identical across plugin-react 4.x/Vite 6 and 6.x/Vite 8). +// +// The bug (issue #110): the bundler minifies createCallable's `Root` +// function name to a lowercase identifier (e.g. `p`) in the published +// dist, so `type.name` is lowercase and the regex fails — and because +// `type.name` is truthy it shadows `displayName` before it is ever read, +// which is why setting displayName (manually or via react-call/vite) does +// not help. The fix forces an uppercase `.name` on the returned Callable. +function isLikelyComponentTypeFunctionBranch(type: unknown): boolean { + if (typeof type !== 'function') return false + const fn = type as { prototype?: object; name?: string; displayName?: string } + if (fn.prototype != null) { + const proto = fn.prototype as { isReactComponent?: unknown } + if (proto.isReactComponent) return true + const ownNames = Object.getOwnPropertyNames(fn.prototype) + if (ownNames.length > 1 || ownNames[0] !== 'constructor') return false + if (Object.getPrototypeOf(fn.prototype) !== Object.prototype) return false + } + const name = fn.name || fn.displayName + return typeof name === 'string' && /^[A-Z]/.test(name) +} + +describe('Fast Refresh compatibility of the built dist — issue #110', () => { + it('a Callable with no displayName is accepted as a component type', () => { + const Confirm = createCallable<{ message: string }, boolean>(() => null) + expect(Confirm.name).toMatch(/^[A-Z]/) + expect(isLikelyComponentTypeFunctionBranch(Confirm)).toBe(true) + }) + + it('a Callable stays accepted after a displayName is assigned', () => { + const Confirm = createCallable<{ message: string }, boolean>(() => null) + Confirm.displayName = 'Confirm' + expect(isLikelyComponentTypeFunctionBranch(Confirm)).toBe(true) + }) +}) diff --git a/vitest.config.ts b/vitest.config.ts index 70f5af5..8923def 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -3,6 +3,7 @@ import { defineConfig } from 'vitest/config' const DIST_TESTS = [ 'packages/*/src/**/bundle-content.test.ts', 'packages/*/src/**/tarball-snapshot.test.ts', + 'packages/*/src/**/fast-refresh-compat.test.ts', ] export default defineConfig({