Fix #178: recompute usesJsFn after the reachability sweep (no orphan JsFn.res)

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<T>(): BoxOf<T>`) 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

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<T>(): Box<T>`
+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

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<value>
+        for (const f of functions) { pushRoot(f.ir.sig); pushRoot(f.ir.value); pushRoot(f.ir.context) } // context: React.Context.t<value>
         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<T>(): Box<T>` 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

test/golden/cases/jsfn-function-index-reachable/expected/DemoBindings.res

@@ -0,0 +1 @@
+@module("demo") external useReg: (JsfnFunctionIndexReachableTypes.reg) => unit = "useReg"

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"

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 = ""

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)_ 🎉
+

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;

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"

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)_ 🎉
+

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<T>(): Box<T>` 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<T> { cb: Function; v: T }
+export declare function makeBox<T>(): Box<T>;
+export declare function ping(): void;