-
Notifications
You must be signed in to change notification settings - Fork 37
fix(runtime): register decorators at class-evaluation time, not construction #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,9 +3,18 @@ | |||||||||||||||||||
| * @upgrade, @interceptor, @command, @run decorators. The bridge reads from | ||||||||||||||||||||
| * this registry to dispatch host export calls. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * Decorators run during class evaluation, which happens at module init. | ||||||||||||||||||||
| * The bridge is constructed AFTER all decorators have populated the | ||||||||||||||||||||
| * registry, so reads are safe. | ||||||||||||||||||||
| * Registration timing matters. Method decorators run *before* the class | ||||||||||||||||||||
| * decorator (TC39 applies member decorators first, in source order, then the | ||||||||||||||||||||
| * class decorator last) and none of them can see the constructor yet. So the | ||||||||||||||||||||
| * member decorators defer their registration onto a module-scoped queue; the | ||||||||||||||||||||
| * `@capsule` class decorator — which does have the constructor — flushes that | ||||||||||||||||||||
| * queue via `flushDeferred(ctor)`. Everything therefore lands in the registry | ||||||||||||||||||||
| * synchronously at class-evaluation time, before any instance is constructed | ||||||||||||||||||||
| * and before the bridge ever reads it. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * (Earlier this was done from `context.addInitializer`, which only runs at | ||||||||||||||||||||
| * *construction* time — but the bridge reads the registry before constructing | ||||||||||||||||||||
| * anything, so every registration looked empty. See sdk-js#20.) | ||||||||||||||||||||
| */ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export type CapsuleConstructor = new () => object; | ||||||||||||||||||||
|
|
@@ -78,89 +87,102 @@ export function registerCapsule(ctor: CapsuleConstructor, description?: string): | |||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** | ||||||||||||||||||||
| * Decorators may run before `@capsule` if class fields appear before the | ||||||||||||||||||||
| * class itself in source. Buffer pending entries on the constructor so the | ||||||||||||||||||||
| * eventual @capsule call picks them up. | ||||||||||||||||||||
| * Member decorators (@tool/@interceptor/@command/@install/@upgrade/@run) run | ||||||||||||||||||||
| * before the class exists, so they cannot name the constructor. Each defers a | ||||||||||||||||||||
| * closure here; `@capsule` runs last and flushes them with the real ctor. | ||||||||||||||||||||
| * | ||||||||||||||||||||
| * Deferred closures are keyed by the class's shared `context.metadata` object | ||||||||||||||||||||
| * (the same reference reaches every decorator of one class), so members of a | ||||||||||||||||||||
| * class that is never `@capsule`-decorated cannot leak into the next capsule | ||||||||||||||||||||
| * evaluated in the same module. The metadata object is only available under | ||||||||||||||||||||
| * TC39 decorator metadata; where the runtime does not provide it (undefined), | ||||||||||||||||||||
| * we fall back to a single module queue — which is exactly right for the | ||||||||||||||||||||
| * enforced "one @capsule class per module" case. | ||||||||||||||||||||
| */ | ||||||||||||||||||||
| const pendingByCtor = new WeakMap<CapsuleConstructor, CapsuleRegistration>(); | ||||||||||||||||||||
| type DeferredRecord = (ctor: CapsuleConstructor) => void; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| function ensureRegistration(ctor: CapsuleConstructor): CapsuleRegistration { | ||||||||||||||||||||
| if (registration !== undefined && registration.ctor === ctor) { | ||||||||||||||||||||
| return registration; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| let pending = pendingByCtor.get(ctor); | ||||||||||||||||||||
| if (pending === undefined) { | ||||||||||||||||||||
| pending = newRegistration(ctor, undefined); | ||||||||||||||||||||
| pendingByCtor.set(ctor, pending); | ||||||||||||||||||||
| let deferred: DeferredRecord[] = []; | ||||||||||||||||||||
| let deferredByMetadata = new WeakMap<object, DeferredRecord[]>(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function defer(metadata: object | undefined, record: DeferredRecord): void { | ||||||||||||||||||||
| if (metadata === undefined) { | ||||||||||||||||||||
| deferred.push(record); | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return pending; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** Adopt any decorator entries buffered before @capsule fired. */ | ||||||||||||||||||||
| export function adoptPending(ctor: CapsuleConstructor): void { | ||||||||||||||||||||
| if (registration === undefined || registration.ctor !== ctor) return; | ||||||||||||||||||||
| const pending = pendingByCtor.get(ctor); | ||||||||||||||||||||
| if (pending === undefined) return; | ||||||||||||||||||||
| for (const [name, entry] of pending.tools) registration.tools.set(name, entry); | ||||||||||||||||||||
| for (const [topic, entry] of pending.interceptors) registration.interceptors.set(topic, entry); | ||||||||||||||||||||
| for (const [name, entry] of pending.commands) registration.commands.set(name, entry); | ||||||||||||||||||||
| if (pending.installMethod !== undefined && registration.installMethod === undefined) { | ||||||||||||||||||||
| registration.installMethod = pending.installMethod; | ||||||||||||||||||||
| let list = deferredByMetadata.get(metadata); | ||||||||||||||||||||
| if (list === undefined) { | ||||||||||||||||||||
| list = []; | ||||||||||||||||||||
| deferredByMetadata.set(metadata, list); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (pending.upgradeMethod !== undefined && registration.upgradeMethod === undefined) { | ||||||||||||||||||||
| registration.upgradeMethod = pending.upgradeMethod; | ||||||||||||||||||||
| list.push(record); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /** Apply the deferred member registrations for this class against its ctor. */ | ||||||||||||||||||||
| export function flushDeferred(ctor: CapsuleConstructor, metadata?: object): void { | ||||||||||||||||||||
| let pending: DeferredRecord[] = []; | ||||||||||||||||||||
| if (metadata !== undefined) { | ||||||||||||||||||||
| pending = deferredByMetadata.get(metadata) ?? []; | ||||||||||||||||||||
| deferredByMetadata.delete(metadata); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (pending.runMethod !== undefined && registration.runMethod === undefined) { | ||||||||||||||||||||
| registration.runMethod = pending.runMethod; | ||||||||||||||||||||
| if (deferred.length > 0) { | ||||||||||||||||||||
| // Fallback bucket (runtimes without decorator metadata). | ||||||||||||||||||||
| pending = pending.concat(deferred); | ||||||||||||||||||||
| deferred = []; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| if (pending.description !== undefined && registration.description === undefined) { | ||||||||||||||||||||
| registration.description = pending.description; | ||||||||||||||||||||
| for (const record of pending) record(ctor); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+102
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a single global To isolate deferred registrations per class, we can leverage type DeferredRecord = (ctor: CapsuleConstructor) => void;
let deferred: DeferredRecord[] = [];
let pendingByMetadata = new WeakMap<object, DeferredRecord[]>();
export function defer(metadata: object | undefined, record: DeferredRecord): void {
if (!metadata) {
deferred.push(record);
return;
}
let list = pendingByMetadata.get(metadata);
if (!list) {
list = [];
pendingByMetadata.set(metadata, list);
}
list.push(record);
}
/** Apply every deferred member registration against the now-known ctor. */
export function flushDeferred(ctor: CapsuleConstructor, metadata?: object): void {
let pending: DeferredRecord[] = [];
if (metadata) {
pending = pendingByMetadata.get(metadata) ?? [];
pendingByMetadata.delete(metadata);
}
if (deferred.length > 0) {
pending = pending.concat(deferred);
deferred = [];
}
for (const record of pending) record(ctor);
} |
||||||||||||||||||||
|
|
||||||||||||||||||||
| function requireRegistration(ctor: CapsuleConstructor): CapsuleRegistration { | ||||||||||||||||||||
| if (registration === undefined || registration.ctor !== ctor) { | ||||||||||||||||||||
| throw new Error( | ||||||||||||||||||||
| `Internal: @capsule must register ${ctor.name} before recording its members.`, | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| pendingByCtor.delete(ctor); | ||||||||||||||||||||
| return registration; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function recordTool(ctor: CapsuleConstructor, entry: ToolEntry): void { | ||||||||||||||||||||
| const target = ensureRegistration(ctor); | ||||||||||||||||||||
| const target = requireRegistration(ctor); | ||||||||||||||||||||
| if (target.tools.has(entry.name)) { | ||||||||||||||||||||
| throw new Error(`@tool("${entry.name}") declared twice on ${ctor.name}.`); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| target.tools.set(entry.name, entry); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function recordInterceptor(ctor: CapsuleConstructor, entry: InterceptorEntry): void { | ||||||||||||||||||||
| const target = ensureRegistration(ctor); | ||||||||||||||||||||
| const target = requireRegistration(ctor); | ||||||||||||||||||||
| if (target.interceptors.has(entry.topic)) { | ||||||||||||||||||||
| throw new Error(`@interceptor("${entry.topic}") declared twice on ${ctor.name}.`); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| target.interceptors.set(entry.topic, entry); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function recordCommand(ctor: CapsuleConstructor, entry: CommandEntry): void { | ||||||||||||||||||||
| const target = ensureRegistration(ctor); | ||||||||||||||||||||
| const target = requireRegistration(ctor); | ||||||||||||||||||||
| if (target.commands.has(entry.name)) { | ||||||||||||||||||||
| throw new Error(`@command("${entry.name}") declared twice on ${ctor.name}.`); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| target.commands.set(entry.name, entry); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function recordInstall(ctor: CapsuleConstructor, methodName: string): void { | ||||||||||||||||||||
| const target = ensureRegistration(ctor); | ||||||||||||||||||||
| const target = requireRegistration(ctor); | ||||||||||||||||||||
| if (target.installMethod !== undefined) { | ||||||||||||||||||||
| throw new Error(`Only one @install method allowed on ${ctor.name}.`); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| target.installMethod = methodName; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function recordUpgrade(ctor: CapsuleConstructor, methodName: string): void { | ||||||||||||||||||||
| const target = ensureRegistration(ctor); | ||||||||||||||||||||
| const target = requireRegistration(ctor); | ||||||||||||||||||||
| if (target.upgradeMethod !== undefined) { | ||||||||||||||||||||
| throw new Error(`Only one @upgrade method allowed on ${ctor.name}.`); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| target.upgradeMethod = methodName; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export function recordRun(ctor: CapsuleConstructor, methodName: string): void { | ||||||||||||||||||||
| const target = ensureRegistration(ctor); | ||||||||||||||||||||
| const target = requireRegistration(ctor); | ||||||||||||||||||||
| if (target.runMethod !== undefined) { | ||||||||||||||||||||
| throw new Error(`Only one @run method allowed on ${ctor.name}.`); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
@@ -175,4 +197,6 @@ export function getRegistration(): CapsuleRegistration | undefined { | |||||||||||||||||||
| /** Test-only: reset the registry to a clean state. */ | ||||||||||||||||||||
| export function __resetRegistry(): void { | ||||||||||||||||||||
| registration = undefined; | ||||||||||||||||||||
| deferred = []; | ||||||||||||||||||||
| deferredByMetadata = new WeakMap(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
198
to
202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reset the
Suggested change
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||
| type CapsuleConstructor, | ||||||||||||||||||||||||||||||||||||||||||||||||
| defer, | ||||||||||||||||||||||||||||||||||||||||||||||||
| recordTool, | ||||||||||||||||||||||||||||||||||||||||||||||||
| recordInterceptor, | ||||||||||||||||||||||||||||||||||||||||||||||||
| recordCommand, | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -48,16 +48,16 @@ export function tool(name: string, options: ToolOptions = {}) { | |||||||||||||||||||||||||||||||||||||||||||||||
| if (context.private || context.static) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`@tool("${name}") must be applied to a public instance method.`); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| context.addInitializer(function () { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const ctor = (this as object).constructor as CapsuleConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||
| const methodName = String(context.name); | ||||||||||||||||||||||||||||||||||||||||||||||||
| defer(context.metadata, (ctor) => | ||||||||||||||||||||||||||||||||||||||||||||||||
| recordTool(ctor, { | ||||||||||||||||||||||||||||||||||||||||||||||||
| name, | ||||||||||||||||||||||||||||||||||||||||||||||||
| methodName: String(context.name), | ||||||||||||||||||||||||||||||||||||||||||||||||
| methodName, | ||||||||||||||||||||||||||||||||||||||||||||||||
| mutable: options.mutable === true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| description: options.description, | ||||||||||||||||||||||||||||||||||||||||||||||||
| inputSchema: options.inputSchema, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+51
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -80,14 +80,14 @@ export function interceptor(topic: string, options: InterceptorOptions = {}) { | |||||||||||||||||||||||||||||||||||||||||||||||
| if (context.private || context.static) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`@interceptor("${topic}") must be applied to a public instance method.`); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| context.addInitializer(function () { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const ctor = (this as object).constructor as CapsuleConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||
| const methodName = String(context.name); | ||||||||||||||||||||||||||||||||||||||||||||||||
| defer(context.metadata, (ctor) => | ||||||||||||||||||||||||||||||||||||||||||||||||
| recordInterceptor(ctor, { | ||||||||||||||||||||||||||||||||||||||||||||||||
| topic, | ||||||||||||||||||||||||||||||||||||||||||||||||
| methodName: String(context.name), | ||||||||||||||||||||||||||||||||||||||||||||||||
| methodName, | ||||||||||||||||||||||||||||||||||||||||||||||||
| mutable: options.mutable === true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+83
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -107,14 +107,14 @@ export function command(name: string, options: CommandOptions = {}) { | |||||||||||||||||||||||||||||||||||||||||||||||
| if (context.private || context.static) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`@command("${name}") must be applied to a public instance method.`); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| context.addInitializer(function () { | ||||||||||||||||||||||||||||||||||||||||||||||||
| const ctor = (this as object).constructor as CapsuleConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||
| const methodName = String(context.name); | ||||||||||||||||||||||||||||||||||||||||||||||||
| defer(context.metadata, (ctor) => | ||||||||||||||||||||||||||||||||||||||||||||||||
| recordCommand(ctor, { | ||||||||||||||||||||||||||||||||||||||||||||||||
| name, | ||||||||||||||||||||||||||||||||||||||||||||||||
| methodName: String(context.name), | ||||||||||||||||||||||||||||||||||||||||||||||||
| methodName, | ||||||||||||||||||||||||||||||||||||||||||||||||
| mutable: options.mutable === true, | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+110
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pass
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass
context.metadatatoflushDeferredto isolate and flush only the deferred registrations belonging to this class.