Skip to content

feat(jsruntime): V8 named-import static-method dispatch for Effect.succeed#992

Merged
proggeramlug merged 1 commit into
mainfrom
pr-effect-v8-class
May 18, 2026
Merged

feat(jsruntime): V8 named-import static-method dispatch for Effect.succeed#992
proggeramlug merged 1 commit into
mainfrom
pr-effect-v8-class

Conversation

@proggeramlug
Copy link
Copy Markdown
Contributor

Summary

  • import { Effect } from 'effect'; Effect.succeed(42) returned the literal number 0 instead of an Effect class instance — typeof e === 'number', .pipe undefined, ._tag undefined. After this PR: object / function / Success, matching Node byte-for-byte.
  • Same shape blocked any import { X } from 'v8-fallback-pkg'; X.method(args) pattern where the V8 module's top-level export X is itself a sub-namespace object holding the actual functions (Effect / jose / other internal-tool packages).
  • Companion to fix(codegen): #678 followup — V8 wildcard-namespace member calls #985 (which wired up import * as R from 'ramda'; R.sum(...) — the wildcard-namespace branch). This PR wires up the named-import branch via a new js_call_v8_member_method bridge and a JS_HANDLE_TAG fast path in js_object_get_field_by_name.

Root cause

HIR-lower lifts Effect.succeed(42) to StaticMethodCall { class_name: "Effect", method_name: "succeed" } because Effect is uppercase + imported (a workaround for missing cross-module class metadata at HIR-lower time). The codegen StaticMethodCall arm probes methods.get (miss), namespace_imports.contains("Effect") (false — it's Named, not import * as), falls to double_literal(0.0). The #985 ramda fix patched the namespace branch but couldn't reach this one because Effect is itself an object on the effect module (not a function), so effect.succeed(...) at the root doesn't exist — the actual function lives at effect.Effect.succeed. A different bridge call was needed.

Additionally, even with the call fixed, e.value / e._tag on the returned JS-handle-shaped class instance fell through to the small-handle dispatch (which only knows about Fastify/axios/sqlite), so property reads returned undefined. Method calls were already routed correctly through js_call_method's JS_HANDLE_CALL_METHOD dispatch — only property reads needed the equivalent fix.

What this ships

  1. New js_call_v8_member_method runtime FFI (crates/perry-jsruntime/src/interop.rs). Loads the module, reads the named member as an object, asserts the method is callable, invokes with this bound to the member, marshals args + return through the existing native_to_v8/v8_to_native pair.

  2. New emit_v8_member_method_call codegen helper (crates/perry-codegen/src/expr.rs). Materializes three rodata constants + stack-allocates the args buffer + emits the FFI call. The StaticMethodCall arm probes ctx.import_function_v8_specifiers.get(class_name) after the existing namespace branch.

  3. Aliased-import support (crates/perry/src/commands/compile.rs). Records local → imported in import_function_origin_names when local != imported so import { Effect as Eff } correctly looks up Effect on the namespace.

  4. JS_HANDLE_TAG fast path (crates/perry-runtime/src/object.rs). js_object_get_field_by_name now detects top16 == 0x7FFB at entry and routes through the registered JS_HANDLE_OBJECT_GET_PROPERTY callback. Mirror of the existing js_call_method JS_HANDLE_CALL_METHOD routing.

  5. Regression test. test-files/test_v8_class_instance_return.ts + test-files/fixtures/v8_class_instance_pkg/v8_helper.mjs pin a synthetic import { Thing } from "<v8-module>"; Thing.make(42) shape (Effect/jose distilled, six lines of output, matches Node).

Out of scope: alias-import is wired but not regression-tested. Effect's deeper code paths (Schema.ts ~310th-init, HashRing) still hit #684/#809 under the #321 umbrella.

Test plan

  • import { Effect } from 'effect'; Effect.succeed(42) → matches Node object / function / Success
  • Effect.runSync(Effect.succeed(42)) → 42
  • e.pipe(Effect.map(x => x + 1)) chains correctly (object / function)
  • test-files/test_v8_class_instance_return.ts — six lines, matches Node
  • test-files/test_v8_namespace_call.ts (fix(codegen): #678 followup — V8 wildcard-namespace member calls #985 regression) — still passes
  • test-files/test_issue_678_v8_fallback{,_symbols}.ts — still pass
  • test-files/test_gap_*.ts — same six diffs as main (pre-existing, unrelated)
  • import * as L from 'lodash'; L.sum([1,2,3]) — still 6

…cceed

`import { Effect } from 'effect'; Effect.succeed(42)` returned the literal
number 0 instead of an Effect class instance. Root cause: HIR lifts
`Effect.succeed(42)` to StaticMethodCall (uppercase + imported workaround),
the codegen's StaticMethodCall arm probes methods.get + namespace_imports
+ falls through to double_literal(0.0). The #985 ramda fix wired up the
namespace branch; this fix wires up the named-import branch via a new
js_call_v8_member_method bridge that loads the module, gets the named
member as an object, and calls the method on it. Also adds JS_HANDLE_TAG
fast path to js_object_get_field_by_name so subsequent property reads on
returned class instances reach V8 instead of falling to the small-handle
dispatch.

Bumps version to 0.5.997. Detailed root cause and validation in CHANGELOG.
@proggeramlug proggeramlug merged commit 6572ecd into main May 18, 2026
5 of 9 checks passed
@proggeramlug proggeramlug deleted the pr-effect-v8-class branch May 18, 2026 05:36
proggeramlug added a commit that referenced this pull request May 18, 2026
`Effect.runSync(Effect.succeed(42).pipe(Effect.map(x => x + 1)))` returned
`undefined` (TypeError: f is not a function inside Effect's pipeline)
instead of `43`. PR #992 wired up `Effect.succeed(42)` to
`js_call_v8_member_method`, but the follow-on `Effect.map(fn)` still
failed because the Perry closure argument never crossed into V8 as a
real `v8::Function`.

Two-layer fix:

1. HIR `js_transform`: extend the closure -> `JsCreateCallback` rewrite
   to `StaticMethodCall` args when the class name is a JS-imported value
   (mirrors the existing `JsCallMethod` / `JsCallFunction` arms). This
   catches inline-Closure arg patterns like `Effect.map((x) => x + 1)`.

2. Bridge `native_object_to_v8`: add a `GC_TYPE_CLOSURE` arm gated on
   `CLOSURE_MAGIC` that wraps the closure in a `v8::Function` via a new
   `perry_closure_v8_trampoline` (closure pointer stashed in
   `v8::External`, body invokes `js_closure_call_array`). This catches
   the LocalGet / FuncRef path where the HIR transform only sees the
   variable, not the closure literal.

Bumps version to 0.5.1002. Detailed root cause and validation in
CHANGELOG.

Fixture: test-files/test_effect_pipe_map.ts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant