diff --git a/CHANGELOG.md b/CHANGELOG.md index 4505fb0..60258dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ This project adheres to [Semantic Versioning](https://semver.org/). ### Fixed +- **Speculative `@unboxed` classification no longer leaves an orphan `JsFn.res`** (#178) — the + reachability sweep (#191) already drops orphan records stranded by a discarded speculative build, but + a bare `Function` in such a build sets `shared.usesJsFn` (a `JsFn.t` raw node the key-based sweep can't + see), which left an unreferenced `JsFn.res`. `usesJsFn` is now recomputed from the surviving roots + + entries after the sweep, so the emitted file set stays orphan-free. + - **Return-only generic no longer strands an orphan record** (#191) — a function whose type parameter appears only in its return (`jsonBoxed(): BoxOf`) has that return flagged (it can't round-trip), but the record `classify` registered while building the discarded return was left in the output as an diff --git a/docs/TYPE_MAPPING.md b/docs/TYPE_MAPPING.md index f18d31a..058748d 100644 --- a/docs/TYPE_MAPPING.md +++ b/docs/TYPE_MAPPING.md @@ -715,6 +715,14 @@ roots, not just ctor/methods/getters; the `class-setter-static-reachable` fixtur Fixtures: [`return-only-generic-orphan`](../test/golden/cases/return-only-generic-orphan), [`class-setter-static-reachable`](../test/golden/cases/class-setter-static-reachable). (#191) +`shared.usesJsFn` is recomputed from the survivors AFTER the sweep (#178). `JsFn.t` (from a bare +`Function`) is a hand-authored raw node, not a keyed registry entry, so the sweep can't see it — and a +bare `Function` sets `usesJsFn` while building a type that may later be discarded (`makeBox(): Box` +flags its return, dropping the `Box` record and its `cb: Function` field). Left alone, `usesJsFn` would +stay set and emit an unreferenced `JsFn.res`; scanning the surviving roots + entries for a `JsFn.t` node +and clearing the flag when none remain keeps the emitted file set orphan-free. Fixture: +[`speculative-jsfn-orphan`](../test/golden/cases/speculative-jsfn-orphan). (#178) + `opaqueUnion` also gained the **structural dedup** the other builders always had (#180): it registered unconditionally, so two `type.id`s widening to the same module each got their own — blend emitted `ColumnDefinition` and `ColumnDefinition2` with byte-identical bodies. Each `module` declares its own diff --git a/src/extract.mjs b/src/extract.mjs index 357118e..7d88d0b 100644 --- a/src/extract.mjs +++ b/src/extract.mjs @@ -834,6 +834,18 @@ function collectAllRefKeys(node, out, seen) { } } +/** Does an IR tree reach a `JsFn.t` node? `JsFn.t` is a hand-authored raw node (`{kind:'raw', + * res:'JsFn.t'}`, from a bare `Function`), NOT a keyed registry ref — so the reachability sweep can't + * see it. Used to recompute `usesJsFn` from the SURVIVORS after the sweep, so a `JsFn.t` stranded in a + * dropped orphan doesn't leave an unreferenced `JsFn.res`. Cycle-safe. (#178) */ +function irUsesJsFn(node, seen) { + if (!node || typeof node !== 'object' || seen.has(node)) return false + seen.add(node) + if (node.kind === 'raw' && typeof node.res === 'string' && node.res.includes('JsFn')) return true + for (const k in node) { const v = node[k]; if (v && typeof v === 'object' && irUsesJsFn(v, seen)) return true } + return false +} + /** Post-traversal reachability sweep (#191, #178). After all IR trees are final but BEFORE naming is * stabilized, drop any registered entry UNREACHABLE from the emitted roots (component props, function * signatures, class members). A speculative build that bailed — a return-only generic whose return was @@ -3007,25 +3019,43 @@ export function extractModule(entryFile, opts = {}) { // Deep-walk every reference-bearing subtree of the emitted units. Props can be inline records // (fields hold refs), a component can spread a shared base (`baseSpreads[].ref`), a function // nests under `sig`/`value` — a shallow field-walk misses these and would drop live types. - const rootKeys = new Set(), seen = new Set() - const addRoot = (n) => collectAllRefKeys(n, rootKeys, seen) + const roots = [] + const pushRoot = (n) => { if (n) roots.push(n) } for (const c of components) { - for (const p of c.ir.props || []) addRoot(p.type) - for (const b of c.ir.baseSpreads || []) addRoot(b.ref) + for (const p of c.ir.props || []) pushRoot(p.type) + for (const b of c.ir.baseSpreads || []) pushRoot(b.ref) } - for (const f of functions) { addRoot(f.ir.sig); addRoot(f.ir.value); addRoot(f.ir.context) } // context: React.Context.t + for (const f of functions) { pushRoot(f.ir.sig); pushRoot(f.ir.value); pushRoot(f.ir.context) } // context: React.Context.t for (const c of classes) { - addRoot(c.ir.ctor) - for (const m of c.ir.methods || []) addRoot(m) - for (const g of c.ir.getters || []) addRoot(g.type) + pushRoot(c.ir.ctor) + for (const m of c.ir.methods || []) pushRoot(m) + for (const g of c.ir.getters || []) pushRoot(g.type) // A type reachable ONLY through a write-only setter or a static member must count as a root // too — each emits a `@set`/`@scope` external — else its shared type is dropped and the // emitted external dangles. (Getters+setters usually pair up, hiding this behind the getter.) - for (const s of c.ir.setters || []) addRoot(s) - for (const m of c.ir.staticMethods || []) addRoot(m) - for (const v of c.ir.staticValues || []) addRoot(v) + for (const s of c.ir.setters || []) pushRoot(s) + for (const m of c.ir.staticMethods || []) pushRoot(m) + for (const v of c.ir.staticValues || []) pushRoot(v) } + const rootKeys = new Set(), seen = new Set() + for (const r of roots) collectAllRefKeys(r, rootKeys, seen) sweepUnreachableEntries(shared, rootKeys) + // #178: `JsFn.t` (a raw node from a bare `Function`, not a keyed ref) can be stranded when its + // owner entry is swept — e.g. `makeBox(): Box` flags its return, and the `Box` record with + // a `cb: Function` field is dropped, but `usesJsFn` stayed set. Recompute it from the SURVIVORS + // (roots + remaining entries) so a dropped `JsFn.t` doesn't leave an orphan `JsFn.res`. + if (shared.usesJsFn) { + const s2 = new Set() + // `entryChildTypes` returns a record's FIELDS but not its `indexValue` (the `@set_index` + // value type, emit.mjs), so a `Function`-typed index signature — `[k: string]: Function` -> + // `@set_index …Set: (rec, string, JsFn.t)` — would be missed and drop a still-needed JsFn.res. + const survivors = [ + ...roots, + ...shared.entries.flatMap(entryChildTypes), + ...shared.entries.map((e) => e.indexValue).filter(Boolean), + ] + shared.usesJsFn = survivors.some((t) => irUsesJsFn(t, s2)) + } } // #90 residual: give same-base distinct shapes an order-INDEPENDENT intrinsic name, then resync diff --git a/test/golden/cases/jsfn-function-index-reachable/expected/DemoBindings.res b/test/golden/cases/jsfn-function-index-reachable/expected/DemoBindings.res new file mode 100644 index 0000000..b829686 --- /dev/null +++ b/test/golden/cases/jsfn-function-index-reachable/expected/DemoBindings.res @@ -0,0 +1 @@ +@module("demo") external useReg: (JsfnFunctionIndexReachableTypes.reg) => unit = "useReg" diff --git a/test/golden/cases/jsfn-function-index-reachable/expected/JsFn.res b/test/golden/cases/jsfn-function-index-reachable/expected/JsFn.res new file mode 100644 index 0000000..7089193 --- /dev/null +++ b/test/golden/cases/jsfn-function-index-reachable/expected/JsFn.res @@ -0,0 +1,12 @@ +// The bare, untyped JS `Function` (#120) — a callable runtime leaf with no typed signature. +// Construct with the arity matching your callback, read back with the matching `asFnN`. +// Zero-cost: the function passes through unchanged. +type t +external fromFn0: (unit => 'a) => t = "%identity" +external fromFn1: ('a => 'b) => t = "%identity" +external fromFn2: (('a, 'b) => 'c) => t = "%identity" +external fromFn3: (('a, 'b, 'c) => 'd) => t = "%identity" +external asFn0: t => (unit => 'a) = "%identity" +external asFn1: t => ('a => 'b) = "%identity" +external asFn2: t => (('a, 'b) => 'c) = "%identity" +external asFn3: t => (('a, 'b, 'c) => 'd) = "%identity" diff --git a/test/golden/cases/jsfn-function-index-reachable/expected/JsfnFunctionIndexReachableTypes.res b/test/golden/cases/jsfn-function-index-reachable/expected/JsfnFunctionIndexReachableTypes.res new file mode 100644 index 0000000..1d6c3e2 --- /dev/null +++ b/test/golden/cases/jsfn-function-index-reachable/expected/JsfnFunctionIndexReachableTypes.res @@ -0,0 +1,4 @@ +type reg = { + cb: unit => unit, +} +@set_index external regSet: (reg, string, JsFn.t) => unit = "" diff --git a/test/golden/cases/jsfn-function-index-reachable/expected/_REPORT.md b/test/golden/cases/jsfn-function-index-reachable/expected/_REPORT.md new file mode 100644 index 0000000..f4a7678 --- /dev/null +++ b/test/golden/cases/jsfn-function-index-reachable/expected/_REPORT.md @@ -0,0 +1,46 @@ +# Binding report — `demo` + +**0** components · ✅ **0** usable · 🔍 **0** need review · 🛑 **0** broken + +**1** function binding(s) → `DemoBindings.res`. + +**1** shared types deduplicated into **1** `*Types.res` modules (referenced qualified — no per-file redeclaration). + +## 📦 Dependencies + +| Kind | Package | Provides | Status | +|------|---------|----------|--------| +| required | `@rescript/react + stdlib` | JsxDOM, Dom, React, ReactEvent | ✓ present | +| optional | `rescript-webapi` | File, FileList | ✗ not installed | + +## 🔧 Function bindings + +Standalone function exports, emitted as positional `@module external` bindings in `DemoBindings.res`. + +- `useReg` + +## ✅ Usable + +These compile and every prop is bound type-safely — use them directly. +_(n loose)_ = some props widened to `string`; they still work, just loosely typed. + +_(none)_ + +## ⚪ Loosely typed (widened to `string`) + +These resolved to a real but complex type and were widened to `string` (they compile and work). Grouped by type so you can review each pattern once — confirm `string` is acceptable, or it may deserve a tighter mapping. + +_(none)_ + +## 🔍 Needs review + +A multi-type prop couldn't be auto-discriminated at runtime (e.g. two object shapes), so an `@unboxed` variant won't work and we **refuse to use `%identity`/unsafe casts**. The prop is emitted as a `string` placeholder with an inline `// ⚠️ REVIEW` comment — bind it by hand or fix the type upstream. + +_(none)_ + +## 🛑 Broken — needs serious component change + +These props resolved to `unknown`/`any` (usually a generic `T`). They're emitted as a placeholder so the file still compiles, but **the props will not work as typed** — they need a concrete type upstream, or generic-binding support. + +_(none)_ 🎉 + diff --git a/test/golden/cases/jsfn-function-index-reachable/index.d.ts b/test/golden/cases/jsfn-function-index-reachable/index.d.ts new file mode 100644 index 0000000..cdb2922 --- /dev/null +++ b/test/golden/cases/jsfn-function-index-reachable/index.d.ts @@ -0,0 +1,6 @@ +// A SURVIVING record whose `Function`-typed index signature is its only JsFn.t must keep JsFn.res. +// `[event: string]: Function` -> `@set_index …Set: (reg, string, JsFn.t)`, and that JsFn.t lives in +// the record's `indexValue` slot — which `entryChildTypes` omits — so the post-sweep usesJsFn recompute +// must scan indexValue too, else JsFn.res is dropped and the @set_index external dangles. (#178) +interface Reg { cb: () => void; [event: string]: Function } +export declare function useReg(r: Reg): void; diff --git a/test/golden/cases/speculative-jsfn-orphan/expected/DemoBindings.res b/test/golden/cases/speculative-jsfn-orphan/expected/DemoBindings.res new file mode 100644 index 0000000..ce321d3 --- /dev/null +++ b/test/golden/cases/speculative-jsfn-orphan/expected/DemoBindings.res @@ -0,0 +1,3 @@ +// 🛑 BROKEN: `makeBox` has an `unknown`/`any` in its signature — emitted with `string` placeholder(s) and WON'T WORK. Needs a concrete type upstream. +@module("demo") external makeBox: unit => string = "makeBox" +@module("demo") external ping: unit => unit = "ping" diff --git a/test/golden/cases/speculative-jsfn-orphan/expected/_REPORT.md b/test/golden/cases/speculative-jsfn-orphan/expected/_REPORT.md new file mode 100644 index 0000000..b73bb25 --- /dev/null +++ b/test/golden/cases/speculative-jsfn-orphan/expected/_REPORT.md @@ -0,0 +1,47 @@ +# Binding report — `demo` + +**0** components · ✅ **0** usable · 🔍 **0** need review · 🛑 **0** broken + +**2** function binding(s) → `DemoBindings.res`. + +**0** shared types deduplicated into **0** `*Types.res` modules (referenced qualified — no per-file redeclaration). + +## 📦 Dependencies + +| Kind | Package | Provides | Status | +|------|---------|----------|--------| +| required | `@rescript/react + stdlib` | JsxDOM, Dom, React, ReactEvent | ✓ present | +| optional | `rescript-webapi` | File, FileList | ✗ not installed | + +## 🔧 Function bindings + +Standalone function exports, emitted as positional `@module external` bindings in `DemoBindings.res`. + +- `makeBox` +- `ping` + +## ✅ Usable + +These compile and every prop is bound type-safely — use them directly. +_(n loose)_ = some props widened to `string`; they still work, just loosely typed. + +_(none)_ + +## ⚪ Loosely typed (widened to `string`) + +These resolved to a real but complex type and were widened to `string` (they compile and work). Grouped by type so you can review each pattern once — confirm `string` is acceptable, or it may deserve a tighter mapping. + +_(none)_ + +## 🔍 Needs review + +A multi-type prop couldn't be auto-discriminated at runtime (e.g. two object shapes), so an `@unboxed` variant won't work and we **refuse to use `%identity`/unsafe casts**. The prop is emitted as a `string` placeholder with an inline `// ⚠️ REVIEW` comment — bind it by hand or fix the type upstream. + +_(none)_ + +## 🛑 Broken — needs serious component change + +These props resolved to `unknown`/`any` (usually a generic `T`). They're emitted as a placeholder so the file still compiles, but **the props will not work as typed** — they need a concrete type upstream, or generic-binding support. + +_(none)_ 🎉 + diff --git a/test/golden/cases/speculative-jsfn-orphan/index.d.ts b/test/golden/cases/speculative-jsfn-orphan/index.d.ts new file mode 100644 index 0000000..4dc9b13 --- /dev/null +++ b/test/golden/cases/speculative-jsfn-orphan/index.d.ts @@ -0,0 +1,8 @@ +// A bare `Function` sets `shared.usesJsFn` while a speculative type is built, but that type can then be +// discarded — here `makeBox(): Box` flags its return-only generic (rule #4), so the `Box` record +// (with a `cb: Function` -> JsFn.t field) is swept as an orphan. `usesJsFn` must be recomputed from the +// survivors so no unreferenced `JsFn.res` is emitted. The complete file set is the assertion: only +// DemoBindings.res, no JsFn.res, no orphan `*Types.res`. (#178) +interface Box { cb: Function; v: T } +export declare function makeBox(): Box; +export declare function ping(): void;