Repository navigation
fix(runtime): Object.defineProperty on Class.prototype (drizzle applyMixins) — #2159 - #2242
Merged
Merged
Conversation
…the class-vtable / prototype-method side tables (#2159) `Class.prototype` evaluates back to the class-ref itself in Perry (not a separate prototype object), so `Object.defineProperty(C.prototype, name, descriptor)` hit `extract_obj_ptr → null` in `js_object_define_property` and silently dropped the descriptor. Instance lookups (`new C().method`) never saw the entry — drizzle-orm's `applyMixins(MySqlSelectBase, [QueryPromise])` is the load-bearing case: it copies `then`/`catch`/ `finally` from `QueryPromise.prototype` onto `MySqlSelectBase.prototype`, so `await db.select().from(users)` resolved to the builder itself instead of the query result. Fix: detect class-ref receivers up front (using the same `class_ref_id` helper that `js_object_get_own_property_descriptor` / `js_object_get_own_property_names` already consult on class-refs) and install the descriptor's value where instance lookup will find it: * `BOUND_METHOD_FUNC_PTR` closures returned by `getOwnPropertyDescriptor` on a sibling class — extract the source class-ref + method name from the closure captures, look the method up in the source class's vtable chain, and copy the entry onto the target class's vtable. Dispatching the bound closure directly would re-enter `js_native_call_method` against the class-ref (static dispatch arm), so we copy the raw `(func_ptr, param_count)` instead; the regular vtable walk then invokes the method with `this = receiver`. * Any other callable — route through `js_register_prototype_method`, mirroring the existing `Class.prototype.method = fn` direct-assignment path (`lookup_prototype_method` is already consulted on the lookup miss). The drizzle-mysql fixture (#489 acceptance) now uses idiomatic `await db.select().from(users)` form instead of the `.execute()` workaround; byte-identical with Node. New gap test `test_gap_2159_defineproperty_class_prototype.ts` pins both the single-method install and the full `applyMixins` shape, including `await thenable` assimilation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2159 —
Object.defineProperty(C.prototype, name, descriptor)is nowhonored on a class-ref receiver, so the
applyMixins(Target, [Source])pattern works and drizzle-orm's
await db.select().from(users)resolvesthrough the inherited
QueryPromise.thenthenable.Class.prototypeevaluates to the class-ref itself in Perry (not aseparate prototype object — see the existing
class_ref_idarm injs_object_get_own_property_descriptor). Pre-fix the definePropertyruntime helper hit
extract_obj_ptr → nullon that class-ref and silentlydropped the descriptor, so
(new C()).methodcame backundefined.The fix detects the class-ref receiver up front and installs the
descriptor's value where instance lookup will find it:
BOUND_METHOD_FUNC_PTRclosures (whatgetOwnPropertyDescriptorreturns for class methods, hot path for
applyMixins): extract thesource class-ref + method name from the closure's captures, look the
method up in the source class's vtable chain via the existing
lookup_class_method_in_chain, and copy the raw(func_ptr, param_count)entry onto the target class'sCLASS_VTABLE_REGISTRY.Dispatching the bound closure directly would route through
js_native_call_methodon the source class-ref (static-dispatch arm)and never reach the inherited instance method.
{ value: function (…) { … } }):route through
js_register_prototype_method— the sameClass.prototype.m = fndirect-assignment side table thatlookup_prototype_methodalready consults on the instance-lookup misspath.
Test plan
test_gap_2159_defineproperty_class_prototype.tspins the single-method install + full
applyMixinsshape + theawait thenableassimilation. Byte-identical withnode --experimental-strip-types.tests/release/packages/drizzle-mysql/entry.tsswitched from theexplicit-
.execute()workaround to idiomaticawait db.select().from(users)form. Fixture passes (with MySQLrunning on 127.0.0.1; CI wiring is compat: Drizzle + MySQL end-to-end CI job #804).
cargo test --release -p perry-runtime --lib— 735/735 pass.cargo fmt --all -- --checkclean.Related
.execute()workaround in Drizzle: 50-line program against real MySQL via @perry/mysql (followup to #488) #489 (drizzle-mysql fixture).await thenablereturns the thenable itself instead of calling.then(resolve, reject)#586 (js_assimilate_thenable— already in place; this fixis what makes
lookup_class_method_in_chainactually find theinherited
then).