diff --git a/.changeset/mini-lynx-production-readiness.md b/.changeset/mini-lynx-production-readiness.md new file mode 100644 index 0000000..2f876da --- /dev/null +++ b/.changeset/mini-lynx-production-readiness.md @@ -0,0 +1,57 @@ +--- +'@amritk/mini-lynx': minor +--- + +Close the gaps between "the tests pass" and "an app can ship this": errors after +construction, the one unverified engine assumption, and the platform values an +app cannot reach. + +The three share a shape, and it is the shape of everything left on the list — +none of them is a missing feature. Each is a place where **this runtime is the +outermost JavaScript frame**, with native code or the job queue above it, so +there is no caller to hand a problem to and nothing above to notice one. + +- **`setErrorHandler`.** `ErrorBoundary` covers construction, which is all a + component can throw during; it runs once and is finished. Everything after + that ran unguarded into a frame with no contract — a handler throwing inside + the engine's own dispatch is undefined behaviour that ranges from the rest of + the frame's listeners being skipped to the app going down, and a commit + throwing on the promise job queue is an unhandled rejection Lynx's main-thread + context has nobody listening for. Both are now caught and reported, with the + source (`'render' | 'event' | 'gesture' | 'commit' | 'lifecycle'`) attached, + because "something threw" and "the commit threw" lead to different + investigations. Handlers on one `(type, name)` are isolated from each other, + since a component and a `ref` did not choose to share a dispatcher; a failed + commit does not wedge the scheduler, so the next mutation recovers the screen; + and `/recycle`'s `componentAtIndex` answers the engine's own `-1` rather than + unwinding into list layout mid-scroll. With no handler installed it warns, so + a mistake is visible in development with no setup. It does not rethrow, which + is the deliberate part: there is nowhere useful to throw *to*. +- **`setEventTransport`, and `/bridge`.** Event delivery is the one assumption + in this package a device could still disprove — the engine hands back a token + this framework made, and that it does so untouched is read from the engine's + source rather than confirmed on hardware. The failure would be total and + silent: the tree renders, nothing responds to touch. So the listener form is + now a seam with the worklet transport as its default, and `@amritk/mini-lynx/bridge` + ships the fallback the design note has always named — string handler names, + with `dispatchNamedEvent` for the app's forwarder to call once the event has + crossed back from the background thread. A device disagreeing is a startup + line rather than a fork. The cost is stated where it is chosen: a thread hop, + and with it the property that a handler runs in the gesture's own frame. +- **`globalProps()`.** Colour scheme, locale, flags and the reduced-motion + preference arrive from native twice — as `lynx.__globalProps` at startup, and + thereafter through the engine calling a global `updateGlobalProps`. An app can + read the first on its own but cannot usefully own the second: the engine calls + exactly one function, so a component reacting to a theme change would be + hoping it was the one that got there. `renderPage` claims the slot and turns + it into a signal, which also makes the reduced-motion line a one-off rather + than a subscription the app maintains. + +`renderPage` also emits `firstScreen` even when the root component throws. That +event is the platform's cue to stop waiting rather than a report of success, and +a blank screen with a crash report is strictly better than a splash screen that +never dismisses and says nothing. + +The core's gzipped budget moves 5064 → 5414 to cover all of it. None of it is +addable from outside the package, and none of it runs on a day when nothing goes +wrong. diff --git a/docs/mini-lynx-runtime.md b/docs/mini-lynx-runtime.md index ff61f30..01abdd2 100644 --- a/docs/mini-lynx-runtime.md +++ b/docs/mini-lynx-runtime.md @@ -131,10 +131,25 @@ the main thread. That is the single most load-bearing inference in this change, and it is worth flagging its status: the engine-side mechanism is read from the engine's source, but a framework-defined token has not been round-tripped on a physical device by this package. **Prototype it on a device before shipping -anything that depends on it.** If it fails, the fallback is contained and -already understood — register string handlers and own the receiving end by -assigning `lynxCoreInject.tt.publishEvent`, exactly as ReactLynx does, at the -cost of a thread hop. +anything that depends on it.** + +The fallback is no longer only understood — it is shipped. `events/transport.ts` +makes the listener form a seam, `/bridge` implements the string-handler +alternative, and the two together turn "this package is wrong about the engine" +from a fork into a startup line: + +```ts +setEventTransport(namedHandlerTransport) +``` + +What the runtime cannot supply is the wire back. A string handler is routed to +the **background** thread, and this runtime is on the main one, so the event +arrives in the other context and the app forwards it — through +`lynxCoreInject.tt.publishEvent` and a context message, exactly as ReactLynx +does, at the cost of a thread hop. That half depends on the engine version and +on how a bundle is split, which is precisely why it is the app's and not a guess +made here. The property lost with it is the one §4 calls the gift: a handler no +longer runs in the same frame as the gesture. The fake engine **throws** on a function listener rather than accepting one, mirroring `@lynx-js/testing-environment`. A test should fail where a device diff --git a/llms-full.txt b/llms-full.txt index 8d88492..4ac4ee8 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -272,9 +272,45 @@ Two things follow that an app has to know: > engine-side mechanism is read from the engine's source, but a > *framework-defined* token has not been round-tripped on a physical device by > this package. **Prototype events on a device before shipping anything that -> depends on them.** If it fails the fallback is contained — string handlers -> plus `lynxCoreInject.tt.publishEvent`, exactly as ReactLynx does, at the cost -> of a thread hop. +> depends on them.** If it fails, the recovery is a config line rather than a +> fork: `setEventTransport(namedHandlerTransport)` from +> `@amritk/mini-lynx/bridge` binds string handlers instead, with the app +> forwarding them back through `dispatchNamedEvent` — exactly the +> `lynxCoreInject.tt.publishEvent` arrangement ReactLynx uses, at the cost of a +> thread hop. + +## Errors after the build go to `setErrorHandler` + +`ErrorBoundary` catches construction, which is all a component can throw +during — it runs once. After that the runtime is the outermost JavaScript frame +and native code is above it, so a throw in a handler or in the scheduled commit +is caught, reported, and not propagated: + +```ts +setErrorHandler((error, source) => report(error, source)) +// source: 'render' | 'event' | 'gesture' | 'commit' | 'lifecycle' +``` + +With no handler installed it warns. Do not wrap handler bodies in `try`/`catch` +to keep the app alive — that is already the behaviour; wrap them only when the +failure means something specific to that screen. + +## Platform values are `globalProps()` + +Colour scheme, locale, flags and the reduced-motion preference come from native, +and `renderPage` claims the `updateGlobalProps` slot the engine pushes them +through, so they are a signal: + +```tsx +const Screen = () => `screen ${globalProps<{ theme: string }>().theme}`} /> + +// Reduced motion is the one the runtime consults itself, via RouteStack: +effect(() => setReducedMotion(globalProps<{ reduceMotion?: boolean }>().reduceMotion === true)) +``` + +Do not define `updateGlobalProps` yourself — the engine calls exactly one +function for it, and defining a second one silently wins or silently loses +depending on load order. ## Element props @@ -339,12 +375,17 @@ Mutations are committed once per tick: a hundred signal writes cost one | `@amritk/mini-lynx/router` | `createRouter`, `createMemoryHistory`, `RouteView`, `RouteLink`, `matchRoute`, `parseQuery`. Matching is pure arithmetic; history is a seam, and memory is the default because a stack of screens the app holds is genuinely what navigation is here | | `@amritk/mini-lynx/forms` | `createForm` / `Field` / `bindField` / `schemaToValidator`. Which binding a field gets is decided by the type of its **initial value**, not by the element | | `@amritk/mini-lynx/query` | `createQuery` over `@tanstack/query-core` — the same API as `@amritk/mini`'s. Mind the main-thread note above | +| `@amritk/mini-lynx/elements` | `querySelector`, `querySelectorAll`, `invoke` — the engine's UI methods, which are actions rather than attributes | +| `@amritk/mini-lynx/gestures` | `setGestureDetector`, `GestureType` — recogniser *arbitration*, the part ordinary events cannot express | +| `@amritk/mini-lynx/recycle` | `recycle` — ``'s cell recycler. A cell receives **getters**, because it outlives the row it was built for | +| `@amritk/mini-lynx/bridge` | `namedHandlerTransport`, `dispatchNamedEvent` — the fallback event transport. Only if the worklet round trip fails on a device | | `@amritk/mini-lynx/testing` | `createFakeEngine`, `serializeTree` — a complete in-memory PAPI | -There is no `/ui`, no `/platform`, no `/gestures`, no `/animate` and no host -subpath. Components are Lynx elements and CSS; environment questions go to -`SystemInfo` and `globalProps`; gestures and animation belong to the engine -(`@keyframes`, transitions, `element.animate()`). +There is no `/ui`, no `/platform`, no `/animate` and no host subpath. Components +are Lynx elements and CSS; environment questions go to `SystemInfo` and +`globalProps()`; animation belongs to the engine (`@keyframes`, transitions, +`element.animate()`). `/gestures` is not a gesture *recogniser* library — the +engine recognises, and that subpath only composes recognisers. ## Testing with `@amritk/mini-lynx/testing` diff --git a/packages/mini-lynx/AGENTS.md b/packages/mini-lynx/AGENTS.md index 0ef2ab6..0d9b057 100644 --- a/packages/mini-lynx/AGENTS.md +++ b/packages/mini-lynx/AGENTS.md @@ -52,12 +52,15 @@ src/ resolve-class.ts A ClassValue → the single space-joined string __SetClasses wants on-cleanup.ts Teardown registered against the enclosing scope warn.ts Recoverable-mistake reporting, without assuming a console + report-error.ts setErrorHandler + guard — what the engine-facing boundaries catch into + global-props.ts The platform's pushed values, as one signal engine/ element-api.ts LynxElementApi — the whole platform boundary, as a type current-engine.ts setEngine / requireEngine / scheduleFlush / globalEngine index.ts The `/engine` subpath: the boundary on its own events/ worklet-registry.ts Handle→closure table + the `runWorklet` global the engine calls + transport.ts Which listener form `__AddEvent` is given — the one replaceable engine bet style/ apply-style.ts The style channel, and the visibility that shares it to-css-name.ts A style key → its CSS spelling @@ -77,6 +80,7 @@ src/ elements/ querySelector/querySelectorAll and invoke — the engine's UI methods gestures/ setGestureDetector — recogniser composition, the part events cannot express recycle/ recycle — 's cell recycler, the one inverted-ownership path here + bridge/ The fallback event transport: string handlers, with the app owning the wire examples/ js-framework-benchmark/ The keyed benchmark; `bun run bench:reconciler` times it ``` @@ -222,6 +226,38 @@ either. for you. - **No raw-markup sink, ever.** There is no `innerHTML` equivalent anywhere on this boundary, so bound data cannot inject elements. +- **Nothing the engine calls may throw back into it.** Four places in this + package are entered from native or from the job queue rather than from an app's + own call stack: the `runWorklet` dispatch behind every event and gesture, + `/recycle`'s `componentAtIndex` and `enqueueComponent`, the scheduled commit, + and the lifecycle slots in `entry.ts`. An exception leaving any of them unwinds + somewhere with no defined behaviour — on a device that ranges from the rest of + a frame's listeners being skipped to the app going down, and on the job queue + it is an unhandled rejection nothing is listening for. Each one wraps its body + in `guard(...)` from `report-error.ts`, which reports and carries on; + `componentAtIndex` additionally answers `-1`, the engine's own "nothing here". + **Any new engine-called callback owes the same wrapper.** Reporting rather than + rethrowing is the deliberate part: there is nowhere useful to throw *to*, and + a swallow would be the silent failure this package works hardest to avoid. + `ErrorBoundary` is unaffected — it still owns construction, which is the only + thing that has a build to unwind. +- **The listener form is a seam, not a constant.** `add-event.ts` asks + `events/transport.ts` what to hand `__AddEvent` rather than deciding. + That exists because the worklet round-trip is the one engine assumption here + that hardware could still disprove, and the failure is total and silent — so + the recovery has to be a startup line rather than a fork. Do not inline the + worklet handle back into `add-event.ts`, and do not add a second call site for + `registerWorklet` outside the default transport and `/gestures` (whose + callbacks the engine will only take as worklets, so they have no fallback). +- **A global the engine calls by name has exactly one owner.** `runWorklet`, + `renderPage`, `removeComponents` and `updateGlobalProps` are all "the engine + calls this function", so whoever assigns last wins and everyone else is + silently gone. This package claims them in one place each — the registry for + the first, `entry.ts` for the rest — and `updateGlobalProps` is why + `global-props.ts` exists at all: an app cannot own that slot AND let a + component react to a change. `runWorklet` is the one that chains to whatever + was there before, because a page mid-migration may legitimately run two + frameworks. - **Anything that builds a subtree LATER must restore the context frame.** `renderChild`, `list` and `ErrorBoundary`'s retry capture `currentFrame()` when they are called — during the component body, inside whatever provider wraps it @@ -311,6 +347,13 @@ That independence has a cost: **a defect found in one is usually latent in the other.** Both gotchas above were found here and then fixed there. When you fix a bug in this package, go read `../mini/AGENTS.md` and look for the same shape. +The error-reporting seam is the one place that shape does NOT apply, and it is +worth knowing why before porting it across: on the web a throw in a handler +reaches `window.onerror` and a rejected commit reaches `unhandledrejection`, both +of which are defined behaviour with somewhere to report to. Here the frame above +is native and there is no such contract, which is why the guards exist in this +package and not in `mini`. + One thing is deliberately borrowed rather than duplicated: the called-signal scanner. `@amritk/mini`'s is purely syntactic and does not know which runtime the JSX belongs to, so it already catches the identical mistake here — which is diff --git a/packages/mini-lynx/AI.md b/packages/mini-lynx/AI.md index 7e4f700..e31c5e7 100644 --- a/packages/mini-lynx/AI.md +++ b/packages/mini-lynx/AI.md @@ -122,9 +122,45 @@ Two things follow that an app has to know: > engine-side mechanism is read from the engine's source, but a > *framework-defined* token has not been round-tripped on a physical device by > this package. **Prototype events on a device before shipping anything that -> depends on them.** If it fails the fallback is contained — string handlers -> plus `lynxCoreInject.tt.publishEvent`, exactly as ReactLynx does, at the cost -> of a thread hop. +> depends on them.** If it fails, the recovery is a config line rather than a +> fork: `setEventTransport(namedHandlerTransport)` from +> `@amritk/mini-lynx/bridge` binds string handlers instead, with the app +> forwarding them back through `dispatchNamedEvent` — exactly the +> `lynxCoreInject.tt.publishEvent` arrangement ReactLynx uses, at the cost of a +> thread hop. + +## Errors after the build go to `setErrorHandler` + +`ErrorBoundary` catches construction, which is all a component can throw +during — it runs once. After that the runtime is the outermost JavaScript frame +and native code is above it, so a throw in a handler or in the scheduled commit +is caught, reported, and not propagated: + +```ts +setErrorHandler((error, source) => report(error, source)) +// source: 'render' | 'event' | 'gesture' | 'commit' | 'lifecycle' +``` + +With no handler installed it warns. Do not wrap handler bodies in `try`/`catch` +to keep the app alive — that is already the behaviour; wrap them only when the +failure means something specific to that screen. + +## Platform values are `globalProps()` + +Colour scheme, locale, flags and the reduced-motion preference come from native, +and `renderPage` claims the `updateGlobalProps` slot the engine pushes them +through, so they are a signal: + +```tsx +const Screen = () => `screen ${globalProps<{ theme: string }>().theme}`} /> + +// Reduced motion is the one the runtime consults itself, via RouteStack: +effect(() => setReducedMotion(globalProps<{ reduceMotion?: boolean }>().reduceMotion === true)) +``` + +Do not define `updateGlobalProps` yourself — the engine calls exactly one +function for it, and defining a second one silently wins or silently loses +depending on load order. ## Element props @@ -189,12 +225,17 @@ Mutations are committed once per tick: a hundred signal writes cost one | `@amritk/mini-lynx/router` | `createRouter`, `createMemoryHistory`, `RouteView`, `RouteLink`, `matchRoute`, `parseQuery`. Matching is pure arithmetic; history is a seam, and memory is the default because a stack of screens the app holds is genuinely what navigation is here | | `@amritk/mini-lynx/forms` | `createForm` / `Field` / `bindField` / `schemaToValidator`. Which binding a field gets is decided by the type of its **initial value**, not by the element | | `@amritk/mini-lynx/query` | `createQuery` over `@tanstack/query-core` — the same API as `@amritk/mini`'s. Mind the main-thread note above | +| `@amritk/mini-lynx/elements` | `querySelector`, `querySelectorAll`, `invoke` — the engine's UI methods, which are actions rather than attributes | +| `@amritk/mini-lynx/gestures` | `setGestureDetector`, `GestureType` — recogniser *arbitration*, the part ordinary events cannot express | +| `@amritk/mini-lynx/recycle` | `recycle` — ``'s cell recycler. A cell receives **getters**, because it outlives the row it was built for | +| `@amritk/mini-lynx/bridge` | `namedHandlerTransport`, `dispatchNamedEvent` — the fallback event transport. Only if the worklet round trip fails on a device | | `@amritk/mini-lynx/testing` | `createFakeEngine`, `serializeTree` — a complete in-memory PAPI | -There is no `/ui`, no `/platform`, no `/gestures`, no `/animate` and no host -subpath. Components are Lynx elements and CSS; environment questions go to -`SystemInfo` and `globalProps`; gestures and animation belong to the engine -(`@keyframes`, transitions, `element.animate()`). +There is no `/ui`, no `/platform`, no `/animate` and no host subpath. Components +are Lynx elements and CSS; environment questions go to `SystemInfo` and +`globalProps()`; animation belongs to the engine (`@keyframes`, transitions, +`element.animate()`). `/gestures` is not a gesture *recogniser* library — the +engine recognises, and that subpath only composes recognisers. ## Testing with `@amritk/mini-lynx/testing` diff --git a/packages/mini-lynx/README.md b/packages/mini-lynx/README.md index bfd06fd..58b845f 100644 --- a/packages/mini-lynx/README.md +++ b/packages/mini-lynx/README.md @@ -130,6 +130,47 @@ expect(serializeTree(engine.page)).toMatchInlineSnapshot() It records what it was asked to do; it does not lay anything out. Layout belongs to the engine. +## Wiring it up for production + +Four lines, and each one covers something an app cannot add from outside. + +```ts +import { effect, globalProps, renderPage, setErrorHandler, setReducedMotion } from '@amritk/mini-lynx' + +// 1. Where errors go. Install it FIRST, so a failed first build is reported too. +setErrorHandler((error, source) => Sentry.captureException(error, { tags: { source } })) + +// 2. Platform values follow the signal, so this is the only place that reads them. +effect(() => setReducedMotion(globalProps<{ reduceMotion?: boolean }>().reduceMotion === true)) + +// 3. The entry. It claims `updateGlobalProps` and `removeComponents` on the way past. +renderPage(App) +``` + +**Errors after construction reach `setErrorHandler`, not the platform.** +`ErrorBoundary` covers the build, which is all a component can throw during — +it runs once and is done. Everything after that runs with this runtime as the +outermost JavaScript frame and native code above it: a handler throws inside the +engine's own dispatch, and the scheduled commit throws on the promise job queue +where nothing is listening. Both are contained and reported instead, so one +misbehaving handler costs its own event rather than the frame, and a failed +commit does not wedge the scheduler — the next mutation queues a fresh one. +With no handler installed the report goes to `console.warn`, so a mistake is +visible in development with no setup at all. + +**A failed first build still hands over to the platform.** `firstScreen` is the +cue to stop waiting, not a report of success, so `renderPage` emits it even when +the root component threw. A blank screen with a crash report beats a splash +screen that never dismisses and says nothing. + +**`globalProps()` is a signal.** Colour scheme, locale, flags and the +reduced-motion preference arrive from native twice — once as `lynx.__globalProps` +at startup and thereafter through the engine calling a global +`updateGlobalProps`. The engine calls exactly one function for the second, so +exactly one thing in the process may own it; `renderPage` claims it and turns it +into a signal a binding can read. That is what makes the reduced-motion line +above a one-off rather than a subscription the app has to maintain. + ## Subpaths Each is its own module graph, so importing one pulls in none of the others. @@ -138,6 +179,7 @@ Each is its own module graph, so importing one pulls in none of the others. |---|---| | `@amritk/mini-lynx` | `renderPage`, signals, `mount`, `list`, the tree ops, the binds, JSX types | | `/engine` | `LynxElementApi`, `LynxElement`, `setEngine` — the platform boundary on its own | +| `/bridge` | `namedHandlerTransport`, `dispatchNamedEvent` — the fallback event transport | | `/flow` | `Show`, `Switch`/`Match`, `Dynamic`, `For`, `Index` | | `/composition` | `createContext`, `Portal`, `ErrorBoundary` | | `/router` | `createRouter`, `createMemoryHistory`, `RouteView`, `RouteLink`, `matchRoute` | @@ -182,8 +224,18 @@ background thread and push results in. by `RouteStack` and is yours to consult anywhere else, but the runtime cannot read the preference: there is no reduced-motion field on `SystemInfo`, and Lynx has no media queries, so no `prefers-reduced-motion` either. It reaches your -host app natively and you pass it in with `setReducedMotion`, the same way you -pass colour scheme. Everything downstream of that is free. +host app natively and you pass it in with `setReducedMotion` — usually from +`globalProps()`, which is where the platform's other pushed values already live. +Everything downstream of that is free. + +**A worklet-transport failure is recoverable, not fatal.** Event delivery rests +on one inference read from the engine's source rather than confirmed on +hardware, and the symptom if it is wrong is total: the tree renders and nothing +responds to touch. That is why `setEventTransport` exists and why `/bridge` +ships the fallback the design note has always named — but the fallback is a +thread hop, so a handler stops running in the gesture's own frame, and it needs +the app to carry events back from the background context. It is a working +recovery path, not a second first-class transport. See *Before you ship*. Deliberately absent, and not on this list: component lifecycle, datasets, template parts, stylesheet adoption and lazy-bundle queries. None has a caller @@ -205,12 +257,27 @@ the tree renders, the tests pass, and the device does nothing. the engine a token of its own making and installs the `runWorklet` global that resolves it, which is what keeps it compilerless. The engine never looks inside the token, so a framework-defined one should round-trip; that has not been - confirmed on hardware. The fallback is contained and already understood: - register string handlers and own the receiving end by assigning - `lynxCoreInject.tt.publishEvent`, as ReactLynx does, at the cost of a thread hop. + confirmed on hardware. **If it does not, you do not have to fork the package.** + The listener form is a seam: + + ```ts + import { setEventTransport } from '@amritk/mini-lynx' + import { dispatchNamedEvent, namedHandlerTransport } from '@amritk/mini-lynx/bridge' + + setEventTransport(namedHandlerTransport) // before rendering + ``` + + That binds string handler names instead, which the engine routes to the + background thread — so the app owns the wire that carries the event back to + `dispatchNamedEvent`, because how that wire is spelled depends on your Lynx + version and on how your bundle is split. `/bridge` documents the usual + `lynxCoreInject.tt.publishEvent` wiring. The cost is the thread hop, and with + it the main-thread gift: a handler no longer runs in the gesture's own frame. 2. **`/gestures` callbacks go through the same mechanism**, so they carry the same - caveat and will be resolved by the same prototype. `has-react-gesture` is set - because the engine gates its arbiter on that attribute name. + caveat and will be resolved by the same prototype — though not by the same + fallback: `__SetGestureDetector` takes worklet callbacks and nothing else, so + there is no string form to swap to. `has-react-gesture` is set because the + engine gates its arbiter on that attribute name. 3. **`/recycle` implements `componentAtIndex` and `enqueueComponent`.** The protocol — the `update-list-info` inventory, and committing each cell with `{ triggerLayout, operationID, elementID, listID }` so the engine can correlate diff --git a/packages/mini-lynx/package.json b/packages/mini-lynx/package.json index 58a5992..71d02b5 100644 --- a/packages/mini-lynx/package.json +++ b/packages/mini-lynx/package.json @@ -65,6 +65,12 @@ "import": "./dist/jsx-dev-runtime.js", "default": "./dist/jsx-dev-runtime.js" }, + "./bridge": { + "development": "./src/bridge/index.ts", + "types": "./dist/bridge/index.d.ts", + "import": "./dist/bridge/index.js", + "default": "./dist/bridge/index.js" + }, "./engine": { "development": "./src/engine/index.ts", "types": "./dist/engine/index.d.ts", diff --git a/packages/mini-lynx/src/add-event.test.ts b/packages/mini-lynx/src/add-event.test.ts index ce95f2f..6e057bd 100644 --- a/packages/mini-lynx/src/add-event.test.ts +++ b/packages/mini-lynx/src/add-event.test.ts @@ -4,6 +4,7 @@ import { addEvent } from './add-event' import { clearEngine, setEngine } from './engine/current-engine' import type { LynxElement } from './engine/element-api' import { clearWorklets } from './events/worklet-registry' +import { setErrorHandler } from './report-error' import { createFakeEngine, type FakeElement, type FakeEngine } from './testing/create-fake-engine' import { createElement } from './tree' @@ -248,4 +249,41 @@ describe('add-event', () => { expect(fired).toEqual(['other']) }) + + it('keeps one handler throwing from costing the others their event', () => { + const { engine, view } = setup() + const fired: string[] = [] + const reported: string[] = [] + setErrorHandler((_error, source) => reported.push(source)) + + addEvent(view, 'bindEvent', 'tap', () => { + throw new Error('boom') + }) + addEvent(view, 'bindEvent', 'tap', () => fired.push('second')) + + engine.dispatch(fake(view), 'tap') + + // Handlers on one pair did not choose to share a dispatcher — a component + // bound one and a `ref` bound the other — so one of them failing must not + // silently unsubscribe the rest. + expect(fired).toEqual(['second']) + expect(reported).toEqual(['event']) + setErrorHandler(null) + }) + + it('does not let a handler throw into the engine dispatch', () => { + const { engine, view } = setup() + setErrorHandler(() => {}) + + addEvent(view, 'bindEvent', 'tap', () => { + throw new Error('boom') + }) + + // An exception unwinding into native event delivery is undefined behaviour + // that ranges from the rest of the frame being skipped to the app going + // down. `engine.dispatch` goes through the real `runWorklet` indirection, + // so this is the device's path rather than beside it. + expect(() => engine.dispatch(fake(view), 'tap')).not.toThrow() + setErrorHandler(null) + }) }) diff --git a/packages/mini-lynx/src/add-event.ts b/packages/mini-lynx/src/add-event.ts index e5989c3..a5d6c62 100644 --- a/packages/mini-lynx/src/add-event.ts +++ b/packages/mini-lynx/src/add-event.ts @@ -1,6 +1,7 @@ import { requireEngine } from './engine/current-engine' import type { LynxElement } from './engine/element-api' -import { registerWorklet, releaseWorklet, type WorkletHandle } from './events/worklet-registry' +import { type BoundListener, eventTransport } from './events/transport' +import { guard } from './report-error' import type { Dispose } from './types' /** @@ -22,10 +23,20 @@ import type { Dispose } from './types' * global `runWorklet` that this runtime supplies. A raw function is accepted at * bind time and then never invoked on the modern engine, which is the worst * possible failure mode and exactly the one this indirection avoids. See - * `events/worklet-registry.ts`, which is where the interesting part lives. + * `events/worklet-registry.ts`, which is where the interesting part lives, and + * `events/transport.ts` for why the choice of listener form is installable + * rather than written into this file. * * The upshot for a caller is nothing at all: you pass a function, it runs, and * it runs on the main thread in the same frame as the gesture. + * + * ## Handlers are isolated from each other + * + * The fan-out is the reason. Several handlers on one pair is the normal case — + * a component binds `bindtap` and a `ref` binds another — and they did not + * choose to share a dispatcher, so one throwing must not cost the others their + * event. Each runs guarded, and a throw is reported rather than propagated; + * `setErrorHandler` is where those reports go. */ export const addEvent = ( element: LynxElement, @@ -44,13 +55,13 @@ export const addEvent = ( existing.handlers.add(handler) } else { const handlers = new Set([handler]) - const worklet = registerWorklet((event) => { + const bound = eventTransport()((event) => { // Iterate a copy so a handler that detaches itself — or another — cannot // disturb the walk it is being called from. - for (const listener of [...handlers]) listener(event) + for (const listener of [...handlers]) guard('event', () => listener(event)) }) - byElement.set(key, { handlers, worklet }) - engine.__AddEvent(element, type, name, worklet) + byElement.set(key, { handlers, bound }) + engine.__AddEvent(element, type, name, bound.listener) } return () => { @@ -59,7 +70,7 @@ export const addEvent = ( registration.handlers.delete(handler) if (registration.handlers.size === 0) { byElement.delete(key) - releaseWorklet(registration.worklet) + registration.bound.release() engine.__AddEvent(element, type, name, null) } } @@ -68,8 +79,8 @@ export const addEvent = ( /** What this module holds per element and `(type, name)` pair. */ type Registration = { readonly handlers: Set<(event: unknown) => void> - /** The token the engine is holding, so it can be released when the last handler goes. */ - readonly worklet: WorkletHandle + /** What the engine is holding, so it can be released when the last handler goes. */ + readonly bound: BoundListener } /** diff --git a/packages/mini-lynx/src/bridge/index.ts b/packages/mini-lynx/src/bridge/index.ts new file mode 100644 index 0000000..7013dc5 --- /dev/null +++ b/packages/mini-lynx/src/bridge/index.ts @@ -0,0 +1,17 @@ +/** + * `@amritk/mini-lynx/bridge` — the recovery path for event delivery. + * + * One subpath for one job: if the default worklet transport turns out not to + * round-trip on your engine build, this is what you install instead. Nothing + * here is imported by the core, so an app that never needs it never pays for + * it — and an app that does needs one line at startup rather than a fork. + * + * The seam itself (`setEventTransport`, `workletTransport`, `EventTransport`) + * is on the `.` entry, because `add-event` reads it on every binding. + */ +export { + clearNamedHandlers, + dispatchNamedEvent, + HANDLER_PREFIX, + namedHandlerTransport, +} from './named-handlers' diff --git a/packages/mini-lynx/src/bridge/named-handlers.test.ts b/packages/mini-lynx/src/bridge/named-handlers.test.ts new file mode 100644 index 0000000..7966103 --- /dev/null +++ b/packages/mini-lynx/src/bridge/named-handlers.test.ts @@ -0,0 +1,132 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { addEvent } from '../add-event' +import { clearEngine, setEngine } from '../engine/current-engine' +import type { LynxElement } from '../engine/element-api' +import { setEventTransport } from '../events/transport' +import { clearWorklets } from '../events/worklet-registry' +import { setErrorHandler } from '../report-error' +import { createFakeEngine, type FakeElement, type FakeEngine } from '../testing/create-fake-engine' +import { createElement } from '../tree' +import { clearNamedHandlers, dispatchNamedEvent, HANDLER_PREFIX, namedHandlerTransport } from './named-handlers' + +/** + * The fallback is only worth having if it works end to end, so these cases run + * the whole round trip the way a device would: bind through the transport, let + * the engine dispatch to a handler NAME, and hand that name back the way an + * app's forwarder would after the event crossed a thread. The fake records + * string dispatches into `publishedEvents()` precisely because that is where a device + * would have sent them. + */ + +const tick = async (): Promise => { + await Promise.resolve() + await Promise.resolve() +} + +const fake = (element: LynxElement): FakeElement => element as unknown as FakeElement + +const setup = (): { engine: FakeEngine; view: LynxElement } => { + const engine = createFakeEngine() + setEngine(engine.api) + setEventTransport(namedHandlerTransport) + return { engine, view: createElement('view') } +} + +afterEach(async () => { + await tick() + setEventTransport(null) + setErrorHandler(null) + clearNamedHandlers() + clearEngine() + clearWorklets() + // Otherwise a case that counts warnings inherits the previous case's spy, + // and counts its calls too. + vi.restoreAllMocks() +}) + +describe('bridge/named-handlers', () => { + it('binds a handler name the engine will route to the background thread', () => { + const { view } = setup() + + addEvent(view, 'bindEvent', 'tap', () => {}) + + const listener = fake(view).events.get('bindEvent:tap') + expect(typeof listener).toBe('string') + expect(listener as string).toContain(HANDLER_PREFIX) + }) + + it('completes the round trip the way an app forwarder would', () => { + const { engine, view } = setup() + const fired: unknown[] = [] + addEvent(view, 'bindEvent', 'tap', (event) => fired.push(event)) + + engine.dispatch(fake(view), 'tap', { x: 1 }) + + // The engine published to a name rather than calling us: on a device this + // is the point where the event is in the OTHER context. + const published = engine.publishedEvents() + expect(published).toHaveLength(1) + expect(fired).toEqual([]) + + // …and this is the app's forwarder handing it back. + const delivered = dispatchNamedEvent(published[0]?.handler as string, published[0]?.event) + + expect(delivered).toBe(true) + expect(fired).toEqual([{ x: 1 }]) + }) + + it('gives every binding its own name, so two on one element cannot collide', () => { + const { view } = setup() + const other = createElement('view') + + addEvent(view, 'bindEvent', 'tap', () => {}) + addEvent(view, 'bindEvent', 'longpress', () => {}) + addEvent(other, 'bindEvent', 'tap', () => {}) + + const names = [ + fake(view).events.get('bindEvent:tap'), + fake(view).events.get('bindEvent:longpress'), + fake(other).events.get('bindEvent:tap'), + ] + expect(new Set(names).size).toBe(3) + }) + + it('forgets the name once the last handler detaches', () => { + const { view } = setup() + const dispose = addEvent(view, 'bindEvent', 'tap', () => {}) + const name = fake(view).events.get('bindEvent:tap') as string + + dispose() + vi.spyOn(console, 'warn').mockImplementation(() => {}) + + // The table holds nothing an element does not, so a stale name from a + // torn-down screen cannot resurrect a handler. + expect(dispatchNamedEvent(name, {})).toBe(false) + }) + + it('warns rather than throwing on a name it does not know', () => { + setup() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + // By the time this is called the event already crossed a thread and the + // element may be gone. That is an ordinary race, not a mistake. + expect(() => dispatchNamedEvent(`${HANDLER_PREFIX}999`, {})).not.toThrow() + expect(warn).toHaveBeenCalledTimes(1) + }) + + it('reports a throwing handler instead of unwinding into the forwarder', () => { + const { engine, view } = setup() + const reported: string[] = [] + setErrorHandler((_error, source) => reported.push(source)) + addEvent(view, 'bindEvent', 'tap', () => { + throw new Error('boom') + }) + + engine.dispatch(fake(view), 'tap') + const published = engine.publishedEvents() + + expect(() => dispatchNamedEvent(published[0]?.handler as string, published[0]?.event)).not.toThrow() + expect(reported).toEqual(['event']) + }) +}) diff --git a/packages/mini-lynx/src/bridge/named-handlers.ts b/packages/mini-lynx/src/bridge/named-handlers.ts new file mode 100644 index 0000000..e8f5db2 --- /dev/null +++ b/packages/mini-lynx/src/bridge/named-handlers.ts @@ -0,0 +1,119 @@ +import type { EventTransport } from '../events/transport' +import { guard } from '../report-error' +import { warn } from '../warn' + +/** + * The fallback event transport: string handler names, with the app owning the + * wire that carries the event back. + * + * ## When you need this + * + * Only when the default does not work on your engine build. The worklet + * transport binds a token this package made and relies on the engine handing it + * back to `runWorklet` untouched — an inference read from the engine's source + * rather than confirmed on hardware. If a device says otherwise, the symptom is + * unmistakable and total: the tree renders correctly and nothing responds to + * touch, with one line in the native log. + * + * That is the failure this module exists for, and swapping is one line: + * + * ```ts + * import { setEventTransport } from '@amritk/mini-lynx' + * import { dispatchNamedEvent, namedHandlerTransport } from '@amritk/mini-lynx/bridge' + * + * setEventTransport(namedHandlerTransport) + * ``` + * + * ## Why the app has to supply the other half + * + * A string listener is a handler NAME, and the engine routes it to the + * **background** thread: it calls `lynxCoreInject.tt.publishEvent(name, event)` + * in the background context. This runtime lives on the main thread, so the + * handler the engine is trying to reach is in the other context and the event + * has to be carried back. + * + * The carrying is the app's, and deliberately so — it is the one part of this + * that depends on your Lynx version and on how your bundle is split, and a + * runtime that hard-coded a guess would fail the same silent way it is trying + * to rescue you from. What this module owns is the part that is the same + * everywhere: naming the handlers, keeping the table, and resolving a name back + * to the closure that was bound. + * + * A typical wiring, with the background chunk forwarding to the main one: + * + * ```ts + * // background chunk — the engine calls this by name + * lynxCoreInject.tt.publishEvent = (name, event) => + * lynx.getCoreContext().dispatchEvent({ type: 'mini-lynx:event', data: { name, event } }) + * + * // main-thread chunk — hand it to the table + * lynx.getJSContext().addEventListener('mini-lynx:event', ({ data }) => + * dispatchNamedEvent(data.name, data.event), + * ) + * ``` + * + * ## What it costs + * + * A thread hop per event, and with it the property this package's design note + * calls the main-thread gift: a handler no longer runs in the same frame as the + * gesture that triggered it. Every synchronous guarantee downstream of that — + * a signal write reaching the tree before the frame commits — goes with it. + * That is why this is the fallback and not the default. + */ + +/** + * The prefix on every handler name this module hands out. + * + * Exported so a forwarder can tell this runtime's events from another + * framework's on the same page, which is the situation a migration lives in for + * as long as it takes. + */ +export const HANDLER_PREFIX = 'mini-lynx:' + +/** Bound dispatchers, keyed by the handler name the engine is holding. */ +const dispatchers = new Map void>() + +let nextId = 1 + +/** + * Binds a listener as a handler NAME rather than as a worklet handle. + * + * Install it with `setEventTransport(namedHandlerTransport)`. Names are unique + * per binding rather than per element or per event type, so two handlers on the + * same element cannot collide, and `release` forgets one the moment its last + * handler detaches — the table holds nothing an element does not. + */ +export const namedHandlerTransport: EventTransport = (dispatch) => { + const name = `${HANDLER_PREFIX}${nextId++}` + dispatchers.set(name, dispatch) + return { listener: name, release: () => dispatchers.delete(name) } +} + +/** + * Delivers an event the engine published to the listener bound under `name`. + * + * Returns whether a listener was found, so a page running more than one + * framework can fall through to the other one instead of dropping the event. + * An unknown name warns rather than throwing: by the time this is called the + * event has already crossed a thread, and the element it was bound to may have + * been removed in between — a stale name is an ordinary race, not a mistake. + * + * The listener is guarded exactly as the worklet path guards its own, because + * the frame above this one is usually the engine's message delivery and a throw + * there is somebody else's undefined behaviour. + */ +export const dispatchNamedEvent = (name: string, event: unknown): boolean => { + const dispatch = dispatchers.get(name) + if (!dispatch) { + warn('bridge: no listener is bound under the handler name:', name) + return false + } + guard('event', () => dispatch(event)) + return true +} + +/** Drops every binding. For tests, which need a clean table per case. */ +export const clearNamedHandlers = (): void => { + dispatchers.clear() + nextId = 1 +} diff --git a/packages/mini-lynx/src/core-size-budget.test.ts b/packages/mini-lynx/src/core-size-budget.test.ts index 915f852..d0e4473 100644 --- a/packages/mini-lynx/src/core-size-budget.test.ts +++ b/packages/mini-lynx/src/core-size-budget.test.ts @@ -32,6 +32,15 @@ import { describe, expect, it } from 'vitest' * the per-element style bookkeeping that keeps `show` alive across a style * write, and the per-tag creator table. * + * It moved a second time, by 350 bytes (5064 → 5414 measured), for the three + * things an app cannot add from outside and should not have to: + * `report-error.ts` and the guards at the boundaries the engine calls in on, + * the event-transport seam that makes the one unverified engine assumption + * replaceable rather than a fork, and `global-props.ts`, which claims a + * lifecycle slot only one thing in the process may claim. Each pays for itself + * on the day something goes wrong on a device, which is the only day any of + * them runs. + * * It stays snug against the measured size on purpose. `/flow` is several times * the headroom and `/testing` is a complete Element PAPI, so a real leak cannot * hide in it. Raise it only for a deliberate, reviewed change to the core. @@ -40,10 +49,10 @@ import { describe, expect, it } from 'vitest' const PKG_ROOT = fileURLToPath(new URL('..', import.meta.url)) /** Gzipped-byte ceiling for the bundled `.` entry. */ -const GZIP_BUDGET = 5200 +const GZIP_BUDGET = 5450 /** Subpath directories whose sources must never enter the core graph. */ -const SUBPATH_DIRS = ['flow/', 'composition/', 'router/', 'testing/', 'forms/', 'query/'] +const SUBPATH_DIRS = ['flow/', 'composition/', 'router/', 'testing/', 'forms/', 'query/', 'bridge/'] const built = await build({ entryPoints: ['src/index.ts'], diff --git a/packages/mini-lynx/src/engine/current-engine.test.ts b/packages/mini-lynx/src/engine/current-engine.test.ts index 6805c28..0802513 100644 --- a/packages/mini-lynx/src/engine/current-engine.test.ts +++ b/packages/mini-lynx/src/engine/current-engine.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it } from 'vitest' +import { setErrorHandler } from '../report-error' import { createFakeEngine } from '../testing/create-fake-engine' import { createElement, setText } from '../tree' import { clearEngine, globalEngine, requireEngine, scheduleFlush, setEngine } from './current-engine' @@ -136,4 +137,52 @@ describe('current-engine', () => { // globals, so there is nothing to import. expect(globalEngine()).toBe(globalThis) }) + + it('reports a commit that throws instead of losing it to the job queue', async () => { + const engine = createFakeEngine() + const failing = { + ...engine.api, + __FlushElementTree: () => { + throw new Error('the engine refused the commit') + }, + } + const reported: string[] = [] + setErrorHandler((_error, source) => reported.push(source)) + setEngine(failing) + + scheduleFlush() + await tick() + + // Unguarded this is an unhandled rejection, and Lynx's main-thread context + // is exactly where nothing is listening for one — the missing commit would + // be the only symptom. + expect(reported).toEqual(['commit']) + setErrorHandler(null) + }) + + it('recovers on the next mutation after a commit failed', async () => { + const engine = createFakeEngine() + let fail = true + const flaky = { + ...engine.api, + __FlushElementTree: (...args: Parameters) => { + if (fail) throw new Error('not this time') + engine.api.__FlushElementTree(...args) + }, + } + setErrorHandler(() => {}) + setEngine(flaky) + + scheduleFlush() + await tick() + + // The flag is cleared before the call rather than after, so one failure + // does not wedge the scheduler and leave the screen frozen for good. + fail = false + scheduleFlush() + await tick() + + expect(engine.flushes()).toBe(1) + setErrorHandler(null) + }) }) diff --git a/packages/mini-lynx/src/engine/current-engine.ts b/packages/mini-lynx/src/engine/current-engine.ts index 491a9cc..6522dc6 100644 --- a/packages/mini-lynx/src/engine/current-engine.ts +++ b/packages/mini-lynx/src/engine/current-engine.ts @@ -1,3 +1,4 @@ +import { guard } from '../report-error' import type { LynxElementApi } from './element-api' /** @@ -95,13 +96,21 @@ export const clearEngine = (): void => { * engine would receive the commit while the incoming one — the only engine with * anything on screen — never got flushed. For the same reason `clearEngine` * leaves the queued flag alone: the pending microtask is what clears it. + * + * The commit is guarded because of where it runs. A throw here escapes into the + * promise job queue and becomes an unhandled rejection — and Lynx's main-thread + * context is exactly the sort of restricted environment where nothing is + * listening for one, so the commit that stopped happening would be the only + * symptom. Worse, the flag is cleared before the call rather than after, so a + * failed commit does not wedge the scheduler: the next mutation queues a fresh + * one and the screen can recover. */ export const scheduleFlush = (): void => { if (flushQueued || !current) return flushQueued = true void Promise.resolve().then(() => { flushQueued = false - current?.__FlushElementTree() + guard('commit', () => current?.__FlushElementTree()) }) } diff --git a/packages/mini-lynx/src/entry.test.ts b/packages/mini-lynx/src/entry.test.ts index effb545..6bd6b3e 100644 --- a/packages/mini-lynx/src/entry.test.ts +++ b/packages/mini-lynx/src/entry.test.ts @@ -2,6 +2,9 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { clearEngine } from './engine/current-engine' import { renderPage } from './entry' +import { globalProps, setGlobalProps } from './global-props' +import { onCleanup } from './on-cleanup' +import { setErrorHandler } from './report-error' import { createFakeEngine } from './testing/create-fake-engine' import { createElement } from './tree' @@ -16,6 +19,8 @@ import { createElement } from './tree' type LynxGlobals = { renderPage?: (data?: unknown) => void removeComponents?: () => void + updateGlobalProps?: (props?: Readonly>) => void + lynx?: { __globalProps?: Readonly> } __OnLifecycleEvent?: (event: { type: string }) => void } @@ -23,8 +28,12 @@ const globals = (): LynxGlobals => globalThis as unknown as LynxGlobals afterEach(() => { clearEngine() + setErrorHandler(null) + setGlobalProps({}) delete globals().renderPage delete globals().removeComponents + delete globals().updateGlobalProps + delete globals().lynx delete globals().__OnLifecycleEvent }) @@ -91,6 +100,79 @@ describe('entry', () => { // reporting the timing. expect(() => globals().renderPage?.()).not.toThrow() }) + + it('still reports the first screen when the build throws', () => { + const engine = createFakeEngine() + const seen: string[] = [] + const reported: string[] = [] + globals().__OnLifecycleEvent = (event) => seen.push(event.type) + setErrorHandler((_error, source) => reported.push(source)) + + renderPage( + () => { + throw new Error('the root component threw') + }, + { engine: engine.api }, + ) + + // `firstScreen` is the platform's cue to stop waiting, not a report of + // success. Skipping it leaves the app on its splash screen forever with + // nothing anywhere saying why — strictly worse than a blank screen and a + // crash report. + expect(() => globals().renderPage?.()).not.toThrow() + expect(seen).toEqual(['firstScreen']) + expect(reported).toEqual(['render']) + }) + + it('reads the platform values that were already set before the page ran', () => { + const engine = createFakeEngine() + globals().lynx = { __globalProps: { theme: 'dark' } } + + renderPage(() => createElementUnderEngine(engine), { engine: engine.api }) + globals().renderPage?.() + + // Read before the build, so the first frame renders against the real values + // rather than against defaults it then has to correct. + expect(globalProps<{ theme?: string }>().theme).toBe('dark') + }) + + it('claims the slot the engine pushes later platform values through', () => { + const engine = createFakeEngine() + renderPage(() => createElementUnderEngine(engine), { engine: engine.api }) + globals().renderPage?.() + + globals().updateGlobalProps?.({ theme: 'light' }) + + // The engine calls exactly one function for this, so exactly one thing in + // the process may own it. A component that wanted to react to a theme + // change would otherwise be hoping it was the one that got there. + expect(globalProps<{ theme?: string }>().theme).toBe('light') + }) + + it('tears down once even when a cleanup throws', () => { + const engine = createFakeEngine() + const reported: string[] = [] + setErrorHandler((_error, source) => reported.push(source)) + + renderPage( + () => { + onCleanup(() => { + throw new Error('a cleanup threw') + }) + return createElementUnderEngine(engine) + }, + { engine: engine.api }, + ) + globals().renderPage?.() + + expect(() => globals().removeComponents?.()).not.toThrow() + expect(reported).toEqual(['lifecycle']) + + // The engine is about to reuse this context either way, and a root that + // stayed held after a failed teardown is what actually leaks. + expect(() => globals().removeComponents?.()).not.toThrow() + expect(reported).toEqual(['lifecycle']) + }) }) /** Builds one element, with the engine installed by `renderPage` already current. */ diff --git a/packages/mini-lynx/src/entry.ts b/packages/mini-lynx/src/entry.ts index 42fc45d..0214bbc 100644 --- a/packages/mini-lynx/src/entry.ts +++ b/packages/mini-lynx/src/entry.ts @@ -1,6 +1,8 @@ import { globalEngine, setEngine } from './engine/current-engine' import type { LynxElement, LynxElementApi } from './engine/element-api' +import { setGlobalProps } from './global-props' import { mount } from './mount' +import { guard } from './report-error' import type { Dispose } from './types' /** @@ -15,10 +17,25 @@ import type { Dispose } from './types' * startup. Optionally it may also define `processData`, `updatePage`, * `updateGlobalProps`, `getPageData` and `removeComponents`. * - * This runtime needs only the first of those, because it has no data pipeline - * to sit in the middle of: state is signals, and a signal is written by whoever - * owns it rather than pushed in from the platform. So the contract collapses to - * "build the tree once", which is exactly what `mount` already does. + * This runtime needs two of those, and ignores the rest because it has no data + * pipeline to sit in the middle of: state is signals, and a signal is written by + * whoever owns it rather than pushed in from the platform. So the contract + * collapses to "build the tree once", which is exactly what `mount` already + * does, plus the two slots the engine uses to reach the page *later*: + * `removeComponents` before a reload, and `updateGlobalProps` when the platform + * pushes new values. Both are claimed here rather than left to the app because + * the engine calls exactly one function for each — see `global-props.ts` for + * why that makes them the runtime's to own. + * + * ## Why the first screen is announced even when the build fails + * + * `firstScreen` is not a report of success; it is the platform's cue to stop + * waiting. Skipping it after a throw leaves the app on its splash screen + * forever with no error anywhere — the worst of both outcomes, since a failed + * build that still hands over gives you a blank screen you can see, a report you + * can read, and a process you can navigate out of. So the build is guarded, the + * failure is reported through `setErrorHandler`, and the handover happens + * either way. * * ## Why `__OnLifecycleEvent({ type: 'firstScreen' })` matters * @@ -44,20 +61,47 @@ export const renderPage = (component: () => LynxElement, options: RenderPageOpti target.renderPage = () => { setEngine(engine) - // The page element belongs to the engine and already exists by the time - // `renderPage` is called; creating our own root here would build a tree - // nothing is showing. - const page = engine.__GetPageElement?.() ?? engine.__CreateElement('page', 0, {}) - disposeRoot = mount(page, component) - target.__OnLifecycleEvent?.({ type: 'firstScreen' }) + // Whatever the platform already decided before the page ran — colour scheme, + // locale, flags. Read before the build so the first frame renders against + // the real values rather than against defaults it then has to correct. + guard('lifecycle', () => { + // Through `unknown` because `@lynx-js/types` declares `__globalProps` as + // an augmentable interface with no index signature — an app's own shape, + // which is exactly what makes it unassignable to a bag of unknowns here. + const initial = (globalThis as unknown as LynxRuntimeGlobal).lynx?.__globalProps + if (initial) setGlobalProps(initial) + }) + + guard('render', () => { + // The page element belongs to the engine and already exists by the time + // `renderPage` is called; creating our own root here would build a tree + // nothing is showing. + const page = engine.__GetPageElement?.() ?? engine.__CreateElement('page', 0, {}) + disposeRoot = mount(page, component) + }) + + // Outside the build's guard on purpose: the platform is told the page is up + // whether or not the build got there. See the note above. It has a guard of + // its own because this whole function is running inside the engine's call. + guard('lifecycle', () => target.__OnLifecycleEvent?.({ type: 'firstScreen' })) } // A reload tears the tree down before the engine builds the next one. Without // this the old tree's effects keep running against elements nothing renders, - // which is a leak that only shows up after the second reload. + // which is a leak that only shows up after the second reload. A throw in a + // cleanup must not stop the teardown: the engine is about to reuse this + // context either way, and a half-disposed tree is what leaks. target.removeComponents = () => { - disposeRoot?.() + const previous = disposeRoot disposeRoot = undefined + guard('lifecycle', () => previous?.()) + } + + // The platform pushing new values — a theme toggle, a language change, a flag + // flipping. The engine calls one function for this, so the runtime claims it + // and turns it into a signal instead. + target.updateGlobalProps = (next) => { + guard('lifecycle', () => setGlobalProps(next ?? {})) } } @@ -72,9 +116,19 @@ export type RenderPageOptions = { engine?: LynxElementApi } +/** + * The one thing this file reads off the `lynx` object: the platform values that + * were already set before the page ran. Declared here rather than imported so + * the entry point does not depend on a types-only peer being installed. + */ +type LynxRuntimeGlobal = { + lynx?: { __globalProps?: Readonly> } +} + /** The slots the engine calls into, and the lifecycle hook it listens on. */ type LynxGlobals = { renderPage?: (data?: unknown) => void removeComponents?: () => void + updateGlobalProps?: (props?: Readonly>) => void __OnLifecycleEvent?: (event: { type: string }) => void } diff --git a/packages/mini-lynx/src/events/transport.test.ts b/packages/mini-lynx/src/events/transport.test.ts new file mode 100644 index 0000000..4028e36 --- /dev/null +++ b/packages/mini-lynx/src/events/transport.test.ts @@ -0,0 +1,134 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { addEvent } from '../add-event' +import { clearEngine, setEngine } from '../engine/current-engine' +import type { LynxElement } from '../engine/element-api' +import { setErrorHandler } from '../report-error' +import { createFakeEngine, type FakeElement, type FakeEngine } from '../testing/create-fake-engine' +import { createElement } from '../tree' +import { type EventTransport, setEventTransport, workletTransport } from './transport' +import { clearWorklets } from './worklet-registry' + +/** + * The seam exists because event delivery is the one engine assumption in this + * package a device could still disprove. These cases pin the two halves of that: + * the default is unchanged for everybody who never touches it, and an installed + * transport genuinely decides what the engine is handed. + */ + +const tick = async (): Promise => { + await Promise.resolve() + await Promise.resolve() +} + +const fake = (element: LynxElement): FakeElement => element as unknown as FakeElement + +const setup = (): { engine: FakeEngine; view: LynxElement } => { + const engine = createFakeEngine() + setEngine(engine.api) + return { engine, view: createElement('view') } +} + +afterEach(async () => { + await tick() + setEventTransport(null) + setErrorHandler(null) + clearEngine() + clearWorklets() +}) + +describe('events/transport', () => { + it('binds through the worklet transport by default', () => { + const { view } = setup() + + addEvent(view, 'bindEvent', 'tap', () => {}) + + // The default is the whole reason this runtime is compilerless and its + // handlers run in the gesture's own frame. A seam that quietly changed it + // would be a regression dressed as flexibility. + expect(fake(view).events.get('bindEvent:tap')).toMatchObject({ type: 'worklet' }) + }) + + it('hands the engine whatever the installed transport returns', () => { + const { engine, view } = setup() + setEventTransport(() => ({ listener: 'app:handler-1', release: () => {} })) + + addEvent(view, 'bindEvent', 'tap', () => {}) + + // A string listener is the fallback's shape, and the engine accepts it — + // this is what makes /bridge a config change rather than a fork. + expect(fake(view).events.get('bindEvent:tap')).toBe('app:handler-1') + expect(engine.calls()).toContain(`__AddEvent(#${fake(view).id}, "bindEvent", "tap", "app:handler-1")`) + }) + + it('routes the fan-out through the transport, not around it', () => { + const { view } = setup() + const delivered: unknown[] = [] + const transport: EventTransport = (dispatch) => { + delivered.push(dispatch) + return { listener: 'app:handler-1', release: () => {} } + } + setEventTransport(transport) + + const fired: string[] = [] + addEvent(view, 'bindEvent', 'tap', () => fired.push('first')) + addEvent(view, 'bindEvent', 'tap', () => fired.push('second')) + + // One dispatcher per (type, name) whichever transport is installed — the + // engine's single-listener constraint is not the transport's to relax. + expect(delivered).toHaveLength(1) + ;(delivered[0] as (event: unknown) => void)({}) + expect(fired).toEqual(['first', 'second']) + }) + + it('releases the transport binding when the last handler detaches', () => { + const { view } = setup() + const release = vi.fn() + setEventTransport(() => ({ listener: 'app:handler-1', release })) + + const first = addEvent(view, 'bindEvent', 'tap', () => {}) + const second = addEvent(view, 'bindEvent', 'tap', () => {}) + + first() + // Still one handler left, so the engine is still holding this listener. + expect(release).not.toHaveBeenCalled() + + second() + expect(release).toHaveBeenCalledTimes(1) + expect(fake(view).events.has('bindEvent:tap')).toBe(false) + }) + + it('goes back to the default when the transport is cleared', () => { + const { view } = setup() + setEventTransport(() => ({ listener: 'app:handler-1', release: () => {} })) + setEventTransport(null) + + addEvent(view, 'bindEvent', 'tap', () => {}) + + expect(fake(view).events.get('bindEvent:tap')).toMatchObject({ type: 'worklet' }) + }) + + it('leaves listeners already bound alone when the transport changes', () => { + const { view } = setup() + const before = createElement('view') + addEvent(before, 'bindEvent', 'tap', () => {}) + + setEventTransport(() => ({ listener: 'app:handler-1', release: () => {} })) + addEvent(view, 'bindEvent', 'tap', () => {}) + + // Rebinding would mean walking a tree the engine owns. Switching is a + // startup decision, and the documentation says so. + expect(fake(before).events.get('bindEvent:tap')).toMatchObject({ type: 'worklet' }) + expect(fake(view).events.get('bindEvent:tap')).toBe('app:handler-1') + }) + + it('exports the default transport so it can be restored by name', () => { + const { view } = setup() + setEventTransport(() => ({ listener: 'app:handler-1', release: () => {} })) + setEventTransport(workletTransport) + + addEvent(view, 'bindEvent', 'tap', () => {}) + + expect(fake(view).events.get('bindEvent:tap')).toMatchObject({ type: 'worklet' }) + }) +}) diff --git a/packages/mini-lynx/src/events/transport.ts b/packages/mini-lynx/src/events/transport.ts new file mode 100644 index 0000000..aeb90c9 --- /dev/null +++ b/packages/mini-lynx/src/events/transport.ts @@ -0,0 +1,75 @@ +import type { EventListenerValue } from '../engine/element-api' +import { registerWorklet, releaseWorklet } from './worklet-registry' + +/** + * How a listener reaches this runtime — the one engine assumption in the + * package that a device could still disprove, made replaceable. + * + * ## Why this seam exists + * + * Everything else here is written against something the engine documents or the + * types declare. Event delivery is not: `__AddEvent` takes a *worklet handle*, + * the engine hands that token back to a global `runWorklet`, and the inference + * this package rests on is that a token the FRAMEWORK made round-trips through + * a mechanism ReactLynx's compiler usually supplies. That is read from the + * engine's source rather than confirmed on hardware, and it is load-bearing for + * every event in every app built on this runtime. + * + * Before this seam, a build where the inference turned out wrong meant editing + * the package. Now it means installing a different transport at startup: the + * listener form is the only thing that varies, so it is the only thing that has + * to be swappable. `@amritk/mini-lynx/bridge` ships the fallback the design note + * has always named — string handler names, with the app owning the receiving + * end — so the recovery path is code with tests rather than a paragraph. + * + * A transport is called once per `(element, type, name)` that gains its first + * handler, and its `release` runs when the last one detaches. + */ +export type EventTransport = (dispatch: (event: unknown) => void) => BoundListener + +/** What a transport hands back: the value the engine stores, and how to forget it. */ +export type BoundListener = { + /** Passed straight to `__AddEvent`. A worklet handle, or a handler name. */ + readonly listener: EventListenerValue + /** Drops whatever bookkeeping the transport did for this listener. */ + readonly release: () => void +} + +/** + * The default: a worklet handle resolved by this package's own `runWorklet`. + * + * This is the transport that keeps the runtime compilerless and its handlers on + * the main thread, so a tap runs in the same frame as the gesture. Prefer it + * unless a device tells you otherwise. + */ +export const workletTransport: EventTransport = (dispatch) => { + const handle = registerWorklet(dispatch) + return { listener: handle, release: () => releaseWorklet(handle) } +} + +let current: EventTransport = workletTransport + +/** + * Installs the transport every subsequent listener is bound through. + * + * Call it once, before rendering. Listeners already bound keep the transport + * they were bound with — rebinding them would mean walking the tree the engine + * owns, and a mixed tree is not a state worth supporting when the only reason + * to switch is that one of the two does not work at all. + * + * Pass `null` to go back to the default. + * + * @example + * ```ts + * // Only if the worklet round-trip fails on your engine build — see /bridge. + * import { namedHandlerTransport } from '@amritk/mini-lynx/bridge' + * + * setEventTransport(namedHandlerTransport) + * ``` + */ +export const setEventTransport = (transport: EventTransport | null | undefined): void => { + current = transport ?? workletTransport +} + +/** The installed transport. Internal — `add-event.ts` is the only caller. */ +export const eventTransport = (): EventTransport => current diff --git a/packages/mini-lynx/src/events/worklet-registry.ts b/packages/mini-lynx/src/events/worklet-registry.ts index d1750d7..d0f1d46 100644 --- a/packages/mini-lynx/src/events/worklet-registry.ts +++ b/packages/mini-lynx/src/events/worklet-registry.ts @@ -1,3 +1,5 @@ +import { type ErrorSource, guard } from '../report-error' + /** * The handle→closure table behind every event listener, and the global the * engine calls to reach it. @@ -43,8 +45,10 @@ * array, and an options object. That a FRAMEWORK-DEFINED token round-trips * through it unmodified follows from the engine never looking inside `value` — * but it has not been verified end-to-end on a device by this package. If a - * particular engine build turns out to disagree, the fix is contained: swap the - * listener form in `add-event.ts`, keep everything else. + * particular engine build turns out to disagree, nothing here has to change: + * this module is one implementation of `events/transport.ts`, and installing a + * different transport is what an app does instead. `/bridge` ships the one the + * design note has always named. */ /** A token the engine stores and hands back. Opaque to the engine, an index to us. */ @@ -53,8 +57,14 @@ export type WorkletHandle = { readonly value: { readonly id: number } } +/** A dispatcher and the boundary name a throw out of it should be reported under. */ +type Entry = { + readonly dispatch: (event: unknown) => void + readonly source: ErrorSource +} + /** The dispatchers, keyed by the token id the engine holds. */ -const dispatchers = new Map void>() +const dispatchers = new Map() let nextId = 1 let installed = false @@ -67,10 +77,10 @@ let installed = false * bundle's main-thread chunk is evaluated in a context this package does not * control and should not be writing to before it is asked to. */ -export const registerWorklet = (dispatch: (event: unknown) => void): WorkletHandle => { +export const registerWorklet = (dispatch: (event: unknown) => void, source: ErrorSource = 'event'): WorkletHandle => { install() const id = nextId++ - dispatchers.set(id, dispatch) + dispatchers.set(id, { dispatch, source }) return { type: 'worklet', value: { id } } } @@ -88,6 +98,13 @@ export const releaseWorklet = (handle: WorkletHandle): void => { * element that was removed in the same tick as the gesture that hit it, and * throwing out of a native dispatch is a worse outcome than dropping an event * for a node that no longer exists. + * + * A throw from the dispatcher is contained for the same reason, one step + * further out: this function is called BY the engine, so an exception leaving it + * unwinds into native event delivery, where the outcome is undefined and ranges + * from the frame's remaining listeners being skipped to the app going down. It + * is reported instead — see `report-error.ts` for why that is the honest trade + * rather than a swallow. */ const install = (): void => { if (installed) return @@ -98,15 +115,20 @@ const install = (): void => { target.runWorklet = (value: unknown, args?: unknown[]): unknown => { const id = (value as { id?: number } | null)?.id - const dispatch = typeof id === 'number' ? dispatchers.get(id) : undefined - if (dispatch) { - dispatch(args?.[0]) + const entry = typeof id === 'number' ? dispatchers.get(id) : undefined + if (entry) { + guard(entry.source, () => entry.dispatch(args?.[0])) return undefined } // Anything this table does not own is passed on, so a page that also runs // another framework — a migration, an embedded card — keeps working. Ours - // is not the only runtime that may want this global. - return previous?.(value, args) + // is not the only runtime that may want this global. It is guarded too: the + // other framework's throw would unwind through the same native frame. + let result: unknown + guard('event', () => { + result = previous?.(value, args) + }) + return result } } diff --git a/packages/mini-lynx/src/gestures/set-gesture-detector.ts b/packages/mini-lynx/src/gestures/set-gesture-detector.ts index 60b6ecb..15ecd9c 100644 --- a/packages/mini-lynx/src/gestures/set-gesture-detector.ts +++ b/packages/mini-lynx/src/gestures/set-gesture-detector.ts @@ -126,7 +126,10 @@ export const setGestureDetector = (element: LynxElement, detector: GestureDetect const handles: WorkletHandle[] = [] const callbacks: { name: string; callback: WorkletHandle }[] = [] for (const [name, callback] of Object.entries(detector.callbacks ?? {})) { - const handle = registerWorklet((event) => callback(event)) + // Reported as a gesture rather than as an event, because that is what the + // author will be looking for: the two arrive through the same dispatch and + // are otherwise indistinguishable in a crash report. + const handle = registerWorklet((event) => callback(event), 'gesture') handles.push(handle) callbacks.push({ name, callback: handle }) } diff --git a/packages/mini-lynx/src/global-props.test.ts b/packages/mini-lynx/src/global-props.test.ts new file mode 100644 index 0000000..dc46519 --- /dev/null +++ b/packages/mini-lynx/src/global-props.test.ts @@ -0,0 +1,51 @@ +import { afterEach, describe, expect, it } from 'vitest' + +import { globalProps, setGlobalProps } from './global-props' +import { effect, effectScope } from './signals' + +/** + * The values the platform pushes in, as a signal. What is worth pinning is that + * it behaves like every other signal here — a binding that reads it follows a + * change — and that a replacement really replaces, since the engine sends the + * complete bag every time and a merge would leave a removed key behind forever. + */ + +afterEach(() => { + setGlobalProps({}) +}) + +describe('global-props', () => { + it('starts as an empty bag rather than undefined', () => { + // A read before the platform has said anything is the whole of the first + // frame, and on some engine versions rather longer. It should not throw. + expect(globalProps()).toEqual({}) + expect(globalProps<{ theme?: string }>().theme).toBeUndefined() + }) + + it('re-runs a binding when the platform pushes a change', () => { + const seen: unknown[] = [] + const dispose = effectScope(() => { + // A block body rather than an expression one: alien-signals reads an + // effect's return value as its cleanup, and `push` returns a number. + effect(() => { + seen.push(globalProps<{ theme?: string }>().theme) + }) + }) + + setGlobalProps({ theme: 'dark' }) + + // This is the point of it being a signal: a theme toggle reaches the tree + // with nothing threaded through it. + expect(seen).toEqual([undefined, 'dark']) + dispose() + }) + + it('replaces the bag rather than merging into it', () => { + setGlobalProps({ theme: 'dark', locale: 'en' }) + setGlobalProps({ theme: 'light' }) + + // `updateGlobalProps` carries the complete set, so merging would make a + // removed key linger for the life of the app. + expect(globalProps()).toEqual({ theme: 'light' }) + }) +}) diff --git a/packages/mini-lynx/src/global-props.ts b/packages/mini-lynx/src/global-props.ts new file mode 100644 index 0000000..1917293 --- /dev/null +++ b/packages/mini-lynx/src/global-props.ts @@ -0,0 +1,67 @@ +import { signal } from './signals' + +/** + * The values the host app pushed in from native, as one signal the whole tree + * can read. + * + * ## Why the runtime owns this rather than the app + * + * `globalProps` is how a Lynx app hands the page things only the platform + * knows — colour scheme, locale, the reduced-motion preference, feature flags, + * a signed-in user id — and the engine hands them over twice, in two different + * ways. The initial bag is a plain global (`lynx.__globalProps`) readable at + * startup, and every later change arrives by the engine **calling** a global + * `updateGlobalProps` that the framework is expected to define. + * + * An app can read the first of those on its own. It cannot usefully own the + * second: the engine calls one function, so exactly one thing in the process + * may define it, and a component that wanted to react to a theme change would + * have to hope it was the one that got there. Worse, defining it *late* — after + * the entry point has already run — races the engine's first update. + * + * So the runtime claims the slot once, in `renderPage`, and turns it into what + * everything else here already is: a signal. Reading it inside a binding means + * the tree follows the platform without anything being threaded through it. + * + * ```tsx + * type Props = { theme: 'light' | 'dark' } + * + * const Screen = () => `screen ${globalProps().theme}`} /> + * ``` + * + * ## The typing + * + * A type parameter rather than an augmentable interface, because the shape is + * the *app's* — agreed with its own native side — and there is no version of it + * this package could usefully declare. Pass your own type at the read site, or + * wrap it once in a helper of your own and let inference do the rest. + */ +const props = signal>>({}) + +/** + * Reads the current bag. Tracks, so a binding re-runs when the platform pushes + * a change. + * + * The default is an empty object rather than `undefined`, so a read of a + * property is safe before the platform has said anything — which is the whole + * of the first frame, and on some engine versions rather longer. + */ +export const globalProps = = Record>(): Readonly => + props() as Readonly + +/** + * Replaces the bag. + * + * `renderPage` calls this for you — once with `lynx.__globalProps` at startup + * and again for every `updateGlobalProps` the engine sends — so an app normally + * never does. It is exported for the two cases that are not that: a test that + * wants a screen rendered against particular platform values, and an app whose + * entry point is its own rather than this package's. + * + * The bag is replaced wholesale rather than merged, because that is what the + * engine sends: `updateGlobalProps` carries the complete set, and merging would + * make a removed key linger forever. + */ +export const setGlobalProps = (next: Readonly>): void => { + props(next) +} diff --git a/packages/mini-lynx/src/import-boundary.test.ts b/packages/mini-lynx/src/import-boundary.test.ts index ed77553..20c3f75 100644 --- a/packages/mini-lynx/src/import-boundary.test.ts +++ b/packages/mini-lynx/src/import-boundary.test.ts @@ -29,7 +29,18 @@ import { describe, expect, it } from 'vitest' const SRC = fileURLToPath(new URL('.', import.meta.url)) /** The subpath directories — none of these may be reachable from `.`. */ -const SUBPATH_DIRS = ['flow', 'composition', 'router', 'forms', 'query', 'testing', 'elements', 'gestures', 'recycle'] +const SUBPATH_DIRS = [ + 'flow', + 'composition', + 'router', + 'forms', + 'query', + 'testing', + 'elements', + 'gestures', + 'recycle', + 'bridge', +] /** * Drops comments before the specifiers are read. diff --git a/packages/mini-lynx/src/index.ts b/packages/mini-lynx/src/index.ts index 6a7864b..3a8c3a8 100644 --- a/packages/mini-lynx/src/index.ts +++ b/packages/mini-lynx/src/index.ts @@ -67,11 +67,19 @@ export { bindValue } from './bind/bind-value' export { clearEngine, globalEngine, requireEngine, setEngine } from './engine/current-engine' export type { LynxElement, LynxElementApi } from './engine/element-api' export { type RenderPageOptions, renderPage } from './entry' +export { + type BoundListener, + type EventTransport, + setEventTransport, + workletTransport, +} from './events/transport' +export { globalProps, setGlobalProps } from './global-props' export { list } from './list' export { mount, pageElement } from './mount' export { onCleanup } from './on-cleanup' export { reducedMotion, setReducedMotion } from './reduced-motion' export { type ChildFactory, renderChild } from './render-child' +export { type ErrorHandler, type ErrorSource, reportError, setErrorHandler } from './report-error' export { batch, computed, diff --git a/packages/mini-lynx/src/recycle/recycle.test.tsx b/packages/mini-lynx/src/recycle/recycle.test.tsx index ad2f261..7320dd3 100644 --- a/packages/mini-lynx/src/recycle/recycle.test.tsx +++ b/packages/mini-lynx/src/recycle/recycle.test.tsx @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it } from 'vitest' import { clearEngine, setEngine } from '../engine/current-engine' import type { LynxElement } from '../engine/element-api' import { mount } from '../mount' +import { setErrorHandler } from '../report-error' import { signal } from '../signals' import { createFakeEngine, type FakeElement, type FakeEngine } from '../testing/create-fake-engine' import { serializeTree } from '../testing/serialize-tree' @@ -288,4 +289,72 @@ describe('recycle', () => { // here" is safe; reaching into a torn-down pool is not. expect(engine.enterListItemAtIndex(element, 0)).toBe(-1) }) + + it('answers "nothing here" when building a cell throws, instead of unwinding into the engine', () => { + const engine = createFakeEngine() + setEngine(engine.api) + const reported: string[] = [] + setErrorHandler((_error, source) => reported.push(source)) + let list!: LynxElement + mount(engine.pageElement, () => { + list = recycle({ + each: () => rowsOf(3), + itemKey: (row) => row.id, + cell: () => { + throw new Error('the cell threw') + }, + }) + return list + }) + const element = engine.find('list') as FakeElement + + // `componentAtIndex` is called BY the engine, mid-scroll. A throw leaving it + // unwinds into native list layout; -1 is a row that does not appear. + expect(engine.enterListItemAtIndex(element, 0)).toBe(-1) + expect(reported).toEqual(['render']) + setErrorHandler(null) + }) + + it('keeps a failing fill from draining the pool', () => { + const engine = createFakeEngine() + setEngine(engine.api) + setErrorHandler(() => {}) + let explode = false + let list!: LynxElement + mount(engine.pageElement, () => { + list = recycle({ + each: () => rowsOf(3), + itemKey: (row) => row.id, + cell: (item) => ( + + + { + if (explode) throw new Error('a binding threw') + return item().label + }} + /> + + + ), + }) + return list + }) + const element = engine.find('list') as FakeElement + + const sign = engine.enterListItemAtIndex(element, 0) + engine.leaveListItem(element, sign) + const before = engine.findAll('list-item').length + + explode = true + expect(engine.enterListItemAtIndex(element, 1)).toBe(-1) + explode = false + + // The failed fill had already taken the cell out of the pool, and the engine + // will never hand back a cell it was told does not exist — so without the + // re-park a failure repeating every frame allocates a fresh element per row. + expect(engine.enterListItemAtIndex(element, 1)).toBe(sign) + expect(engine.findAll('list-item')).toHaveLength(before) + setErrorHandler(null) + }) }) diff --git a/packages/mini-lynx/src/recycle/recycle.ts b/packages/mini-lynx/src/recycle/recycle.ts index 954eb59..a7918e7 100644 --- a/packages/mini-lynx/src/recycle/recycle.ts +++ b/packages/mini-lynx/src/recycle/recycle.ts @@ -5,6 +5,7 @@ import { currentFrame, withFrame } from '../context-frame' import { requireEngine, scheduleFlush } from '../engine/current-engine' import type { LynxElement } from '../engine/element-api' import { onCleanup } from '../on-cleanup' +import { guard } from '../report-error' import { runDetached } from '../run-detached' import { type Signal, signal } from '../signals' import { createElement } from '../tree' @@ -173,8 +174,23 @@ export const recycle = (props: RecycleProps, attributes: Readonly { + let sign = -1 + guard('render', () => { + sign = fillCellAtIndex(list, listID, cellIndex, operationID) + }) + return sign + } + + /** The actual answer, with `componentAtIndex` owning only what happens when it throws. */ + const fillCellAtIndex = (list: LynxElement, listID: number, cellIndex: number, operationID: number): number => { // Guarded here as well as by the inert callbacks installed on teardown, // because the engine is not obliged to honour `__UpdateListCallbacks` // promptly — it may be mid-scroll, and a request that arrives afterwards @@ -200,15 +216,25 @@ export const recycle = (props: RecycleProps, attributes: Readonly(props: RecycleProps, attributes: Readonly { - if (torn) return - const cell = cells.get(sign) - if (!cell) return - cells.delete(sign) + /** Returns a cell to the pool it came from, which is the only place cells are stored idle. */ + const park = (cell: Cell): void => { const parked = pool.get(cell.reuse) if (parked) parked.push(cell) else pool.set(cell.reuse, [cell]) } + /** Takes a cell back when it scrolls out of range. The engine's other call in, so also guarded. */ + const enqueueComponent = (_list: LynxElement, _listID: number, sign: number): void => { + guard('render', () => { + if (torn) return + const cell = cells.get(sign) + if (!cell) return + cells.delete(sign) + park(cell) + }) + } + const list = engine.__CreateList ? engine.__CreateList(componentIdOf(engine), componentAtIndex, enqueueComponent) : fallbackList() diff --git a/packages/mini-lynx/src/report-error.test.ts b/packages/mini-lynx/src/report-error.test.ts new file mode 100644 index 0000000..a8a3083 --- /dev/null +++ b/packages/mini-lynx/src/report-error.test.ts @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { guard, reportError, setErrorHandler } from './report-error' + +/** + * The reporting seam matters most when everything else has already gone wrong, + * so the cases below are mostly about what it refuses to do: throw, lose the + * original error, or go quiet when nobody installed a handler. + */ + +afterEach(() => { + setErrorHandler(null) + vi.restoreAllMocks() +}) + +describe('report-error', () => { + it('hands the error and its source to the installed handler', () => { + const seen: { error: unknown; source: string }[] = [] + setErrorHandler((error, source) => seen.push({ error, source })) + + const failure = new Error('boom') + reportError(failure, 'event') + + // The source is half the report. "Something threw" is not actionable; "a + // handler threw" and "the commit threw" lead to different investigations. + expect(seen).toEqual([{ error: failure, source: 'event' }]) + }) + + it('warns when no handler is installed, rather than going quiet', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + reportError(new Error('boom'), 'commit') + + // Silence is the failure mode this package works hardest to avoid. A + // mistake should be visible in development with no setup at all. + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0]?.[0]).toContain('commit') + }) + + it('survives a handler that throws, and still surfaces the original', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + setErrorHandler(() => { + throw new Error('the reporter is broken too') + }) + + const original = new Error('boom') + // A throw from in here would land back in the native frame the catch was + // protecting, which is the whole thing this module exists to prevent. + expect(() => reportError(original, 'render')).not.toThrow() + + const logged = warn.mock.calls.map((call) => call[1]) + expect(logged).toContain(original) + }) + + it('goes back to warning when the handler is removed', () => { + const handler = vi.fn() + setErrorHandler(handler) + setErrorHandler(null) + + vi.spyOn(console, 'warn').mockImplementation(() => {}) + reportError(new Error('boom'), 'event') + + expect(handler).not.toHaveBeenCalled() + }) + + describe('guard', () => { + it('reports a throw and says the work did not finish', () => { + const handler = vi.fn() + setErrorHandler(handler) + + const completed = guard('event', () => { + throw new Error('boom') + }) + + expect(completed).toBe(false) + expect(handler).toHaveBeenCalledTimes(1) + }) + + it('says the work finished when nothing threw', () => { + const handler = vi.fn() + setErrorHandler(handler) + + expect(guard('event', () => {})).toBe(true) + expect(handler).not.toHaveBeenCalled() + }) + }) +}) diff --git a/packages/mini-lynx/src/report-error.ts b/packages/mini-lynx/src/report-error.ts new file mode 100644 index 0000000..84a8612 --- /dev/null +++ b/packages/mini-lynx/src/report-error.ts @@ -0,0 +1,110 @@ +import { warn } from './warn' + +/** + * Where a throw that escaped app code was caught, so a report says which of the + * runtime's boundaries it crossed rather than only what went wrong. + * + * These are the four places this runtime is the outermost JavaScript frame — + * the engine called in, or the job queue did, and there is no caller above to + * hand the failure to. + */ +export type ErrorSource = + /** Building the tree: `mount`, or the entry point's first render. */ + | 'render' + /** An event handler, running inside the engine's own dispatch. */ + | 'event' + /** A gesture callback, which reaches us through the same dispatch. */ + | 'gesture' + /** The scheduled commit, running on the promise job queue. */ + | 'commit' + /** A lifecycle slot the engine calls: teardown before a reload, global props arriving. */ + | 'lifecycle' + +/** What an app registers to hear about failures the runtime caught. */ +export type ErrorHandler = (error: unknown, source: ErrorSource) => void + +let handler: ErrorHandler | undefined + +/** + * Routes every error the runtime catches to your crash reporter. + * + * ## Why the runtime catches at all + * + * `ErrorBoundary` covers construction, which is the whole of what a component + * can throw during — a component runs once and is finished. Everything after + * that runs with the runtime as the outermost frame, and the frames above it + * are native: + * + * - a handler throws **inside the engine's dispatch**, and a JavaScript + * exception unwinding into native event delivery is not a defined outcome — + * on a device it ranges from the rest of the frame's listeners never running + * to the app going down; + * - the scheduled commit throws **on the promise job queue**, so it becomes an + * unhandled rejection, and Lynx's main-thread context is exactly the sort of + * restricted environment where nothing is listening for one. + * + * Both are silent by default, which is the failure mode this package works + * hardest to avoid everywhere else. So the runtime contains the throw at each + * of those boundaries and reports it — one screen misbehaves instead of the app + * disappearing, and the report is the thing that tells you it happened. + * + * ## Why it does not rethrow + * + * Because there is nowhere useful to throw *to*. Rethrowing hands the exception + * back to native code that did not ask for one; swallowing it silently would be + * worse than the crash. Reporting is the third option and the only one that + * keeps both the frame and the evidence. + * + * With no handler installed the error goes to `warn`, so a mistake is visible in + * development without any setup. Install one in production and send it wherever + * your other crashes go. + * + * @example + * ```ts + * // Once, at startup — before renderPage, so a failed first build is reported too. + * setErrorHandler((error, source) => Sentry.captureException(error, { tags: { source } })) + * ``` + */ +export const setErrorHandler = (next: ErrorHandler | null | undefined): void => { + handler = next ?? undefined +} + +/** + * Reports an error the runtime caught, without letting the report itself fail. + * + * A handler that throws is a real possibility — it is app code, often the first + * thing to run after something already went wrong — and a throw from in here + * would land back in exactly the native frame the catch was protecting. So the + * reporter is itself guarded, and its failure falls through to `warn` alongside + * the original. + */ +export const reportError = (error: unknown, source: ErrorSource): void => { + if (!handler) { + warn(`unhandled error while handling ${source}:`, error) + return + } + try { + handler(error, source) + } catch (failure) { + warn(`the error handler threw while reporting a ${source} error:`, failure) + warn(`the original ${source} error was:`, error) + } +} + +/** + * Runs `fn`, reporting a throw instead of propagating it. + * + * Returns whether it completed, because two callers need to know: the entry + * point still has to tell the platform the first screen is up even when the + * build failed, and a fan-out of handlers wants to keep going after one of them + * threw. + */ +export const guard = (source: ErrorSource, fn: () => void): boolean => { + try { + fn() + return true + } catch (error) { + reportError(error, source) + return false + } +}