feat(jsruntime): #1021 — Perry-class V8 exposure + Reflect.metadata bridge — NestJS sweep#1117
Merged
Merged
Conversation
…ross-boundary wire
Flips the NestJS smoke from `body={"message":"Cannot GET /ping",...}` (404,
no route registered) to `body=pong` (the route is discovered and the
handler runs).
The previous attempt (#1067) wired the Reflect-metadata bridge JS but
NestJS still saw zero modules at scan time. End-to-end the route never
registered because four separate gaps stacked on top of each other:
1. **V8 class wrapper had no methods on the prototype.** `native_class_to_v8`
returned a `v8::Function` whose `.prototype` was V8's default empty
object, so `Object.getPrototypeOf(new metatype())[name]` (the path
NestJS's `MetadataScanner.getAllMethodNames` and `paths-explorer`
walk) resolved to `undefined`. Now `populate_native_class_v8_prototype`
mirrors each entry in `CLASS_VTABLE_REGISTRY` onto the wrapper's
`.prototype` as a `v8::Function` that re-dispatches through the
vtable's `func_ptr` directly (NOT through `js_native_call_method` —
that path walks `jsval.as_pointer()` on the receiver, which is junk
for a V8 object instance and returned the class_id integer instead
of the method body's return value).
2. **`new metatype()` discarded the prototype chain.** `native_class_constructor`
used to `retval.set(v8::Object::new(scope))`, overriding the implicit
`this` whose `[[Prototype]]` V8 already pointed at the populated
wrapper prototype. Now we return `args.this()` for construct calls.
3. **Class names were lost on the boundary.** `metatype.name` came back
`""`, which made NestJS's `ModuleTokenFactory` hash every module under
the same empty-string token — `modules.size` was always 1 (only the
internal core module won the slot). Added a per-`class_id` →
`String` registry (`CLASS_NAMES`, populated by codegen via the new
`js_register_class_name` FFI), and surface it through V8's
`v8::Function::set_name` so the wrapper carries the user-visible
class name.
4. **`crypto.createHash().digest()` returned `""`.** The V8-side stub
threw away `update()` chunks and returned an empty string —
`ModuleTokenFactory.hashString` hashed every module to the same
`""`. Added `op_perry_hash` (sha1/sha256/sha384/sha512/md5 via
`sha1`/`sha2`/`md-5` crates) and rewrote the stub to mirror the
working `createHmac` path (chunked update → final concat → op
→ encode).
5. **`perf_hooks.performance.now` was undefined.** The default `_ =>
"export default {}"` fallback applied; NestJS's
`ModuleTokenFactory.create()` threw `Cannot read properties of
undefined (reading 'now')` and the dynamic-module compile path
bailed before any user modules were inserted. Added a real
`perf_hooks` stub backed by `Date.now()` (good enough for the
serialization-warning timer NestJS uses it for).
Sub-fixes shipped alongside, all required for the route to land:
- **`reflect-metadata` bridge re-asserted after every module evaluation**
(`poll_pending_module_evaluations` in `interop.rs`). The npm
`reflect-metadata` package (loaded transitively by `@nestjs/common`
via `require("reflect-metadata")`) overwrites `Reflect.defineMetadata`
/`getMetadata` etc. — without this, our wrappers from `js_runtime_init`
are gone by the time the first decorator runs. The bridge JS is
idempotent so re-running on each module evaluation is cheap.
- **Perry closure identity stabilized across V8 crossings**
(`NATIVE_CLOSURE_HANDLES` cache + `__perry_closure_ptr` property).
Decorators that key on `descriptor.value` (the method function) now
see the same `v8::Function` instance as later
`prototype['methodName']` reads — load-bearing for any
`WeakMap`-based metadata store on the V8 side.
- **`v8_to_native_metadata_target` recognizes Perry-closure-wrapped V8
functions** so `descriptor.value` round-trips to the same
POINTER_TAG | ptr identity Perry uses internally — the precondition
for the Perry-side `REFLECT_METADATA` store entry from a V8-side
`Reflect.defineMetadata(...)` to be reachable from a Perry-side
`Reflect.getMetadata(...)`.
Test: `test-files/test_reflect_metadata.ts` exercises
`defineMetadata`/`getMetadata`/`hasMetadata` + the decorator-factory
shape. NestJS sweep: `body={"message":"Cannot GET /ping",...}` (404)
→ `body=pong`.
Deferred follow-ups:
- Methods that read `this` against a V8-allocated instance still
dispatch with `TAG_UNDEFINED` as the receiver — the canonical NestJS
controller-handler shape doesn't use `this` so this matches expected
behavior for the smoke. Properly wiring instance-as-`this` would
require allocating a real Perry `ObjectHeader` for V8 wrapper
instances (or routing through the handle table) — a separate
follow-up.
- Class-ref id `INT32_TAG | class_id` collision in the bridge
(existing pre-fix note in `bridge.rs`) is unchanged.
No version bump or changelog change (external-contributor convention).
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
1 participant
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 the last NestJS surface in #1021. The smoke sweep flips from
body={"message":"Cannot GET /ping","error":"Not Found","statusCode":404}(no route registered) →
body=pong(handler runs).What landed
The prior agent diagnosed this as a Reflect-metadata bridge gap, but the
metadata wiring was already in place — five layered V8-fallback gaps
prevented NestJS from even seeing the AppModule at scan time. All five
are required for the route to register; isolating any one would leave the
sweep at 404.
native_class_to_v8populates
metatype.prototypefrom Perry'sCLASS_VTABLE_REGISTRYsoObject.getPrototypeOf(new metatype())[methodName](NestJS'spaths-explorer/MetadataScannerwalk) resolves to a real function.new metatype()preserves prototype chain.native_class_constructorreturns
args.this()(whose[[Prototype]]V8 already populated)instead of overriding with a fresh empty object.
CLASS_NAMESregistry +js_register_class_namecodegen-emitted FFI, then surface the namethrough
v8::Function::set_name— without this every module hashed tothe same empty-string token under NestJS's
ModuleTokenFactory.crypto.createHash().digest()returns real digests. Addedop_perry_hash(sha1/sha256/sha384/sha512/md5) and rewrote theV8-side stub to mirror the working
createHmacpath.perf_hooks.performance.nowis real. Replaced the default emptystub with a
Date.now()-backed shape — without this NestJS'sModuleTokenFactory.create()threwCannot read properties of undefined (reading 'now')during dynamic-module compile and bailedbefore any user modules were inserted.
Plus three sub-fixes to keep the metadata round-trip clean:
install_reflect_metadata_bridgeis re-run after every moduleevaluation (
reflect-metadataoverwrites our wrappers; idempotentre-install survives that).
(
NATIVE_CLOSURE_HANDLEScache +__perry_closure_ptrproperty), sodescriptor.valueandprototype['methodName']resolve to the sameV8 function — load-bearing for any WeakMap-keyed metadata store.
v8_to_native_metadata_targetrecognizes the new closure-ptr propertyand round-trips through Perry's
POINTER_TAG | ptridentity.Test plan
cargo test --release -p perry-codegen --test manifest_consistency— 4/4 passcargo build --release -p perry-runtime -p perry-stdlib -p perry-jsruntime -p perry/tmp/perry-compat-sweep/nestjswith the@Controller()+@Get('/ping')shape from the task brief) returnsbody=pongtest-files/test_reflect_metadata.ts(added in this PR) exercisesdefineMetadata/getMetadata/hasMetadata + the decorator-factory shape
Deferred
thisagainst a V8-allocated instance stilldispatch with
TAG_UNDEFINEDas the receiver. The canonical NestJScontroller-handler shape (the sweep target) doesn't use
thisso thismatches expected behavior. Properly wiring instance-as-
thiswouldrequire allocating a real Perry
ObjectHeaderfor the V8-wrapperinstances (or routing through the handle table) — separate work item.
INT32_TAG | class_idcollision in the V8 bridge(existing pre-fix caveat in
bridge.rs) is unchanged — out of scopefor compat: async lowering blocks self-fetch (express/fastify/nestjs/hono) #1021.
No version bump or changelog change (external-contributor convention).