Skip to content

fix!: isolate DSL class tags across addons - #67

Open
GrapeBaBa wants to merge 3 commits into
mainfrom
fix/isolate-addon-type-tags
Open

fix!: isolate DSL class tags across addons#67
GrapeBaBa wants to merge 3 commits into
mainfrom
fix/isolate-addon-type-tags

Conversation

@GrapeBaBa

@GrapeBaBa GrapeBaBa commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Motivation

DSL class tags were derived only from @typeName(T). Separate addons commonly
rooted at mod.zig can therefore both contain mod.Counter and receive the
same napi_type_tag. An exported function accepting *Counter could accept an
object from the other addon, pass napi_check_object_type_tag, and reinterpret
unrelated native memory as its own class.

The identity must also remain stable when the same compiled addon is loaded
from two paths. A process-local address isolates addons, but changes across
reloads and does not provide that stability.

Description

  • add zapi.addAddonIdentity for the final addon owner's build.zig

  • generate zapi_addon_identity from the owner's build.zig.zon package name,
    version, fingerprint, and the final compile-step name

  • derive each class tag as:

    FNV-1a-128(package@version#fingerprint::addon::ZigType)
    
  • pass the generated identity through class registration, constructors,
    methods, accessors, function arguments, and return-value materialization

  • require distinct logical addons in one package to use unique compile-step
    names and separate root modules; aliases and installed copies retain the
    identity embedded in the compiled artifact

  • require .identity = @import("zapi_addon_identity") only when a DSL path
    touches classes; function-only modules may continue to use
    js.exportModule(@This(), .{})

  • expose js.NoAddonIdentity for low-level wrapper/conversion calls that cannot
    accept or return DSL classes

  • document Zig 0.16 local development via zig build --fork=/path/to/zapi,
    because .path dependencies do not expose the dependency's build.zig

  • add behavioral coverage for cross-addon rejection and compatibility between
    two loaded copies of one compiled addon

  • add a function-only addon compile fixture that instantiates
    js.exportModule(@This(), .{})

Breaking change

DSL addons that export or exchange js.class values must:

  1. call zapi.addAddonIdentity for the final .node compile step in their
    build.zig
  2. pass .identity = @import("zapi_addon_identity") to js.exportModule

The public low-level wrapFunction, wrapClass, convertArg,
convertReturn, and callAndConvert APIs now take a comptime identity type.
Class-free callers pass js.NoAddonIdentity. Low-level napi.module.register
users and function-only js.exportModule users are unchanged.

Testing

  • pnpm build
  • pnpm test — 118 JS tests plus Zig tests passed
  • zig build -Doptimize=ReleaseSafe
  • zig build -Dtarget=x86_64-linux-gnu
  • zig build test:example_js_dsl -Doptimize=ReleaseSafe
  • zig build test:example_addon_isolation -Doptimize=ReleaseSafe
  • pnpm lint:js
  • zig fmt --check ...
  • external URL/hash consumer smoke test for zapi.addAddonIdentity

@GrapeBaBa
GrapeBaBa force-pushed the fix/isolate-addon-type-tags branch from 9f761f5 to cfd74ce Compare July 28, 2026 01:32
@GrapeBaBa GrapeBaBa changed the title fix: isolate DSL class tags across addons fix!: isolate DSL class tags across addons Jul 28, 2026
@matthewkeil matthewkeil removed this from the mainnet-blst milestone Jul 28, 2026
@GrapeBaBa
GrapeBaBa marked this pull request as ready for review August 10, 2026 10:03

@nazarhussain nazarhussain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one observation.

A package with v1.0.0 and v1.0.1 in the same process would not be able to exchange class objects. While a normal user might think otherway. So we should document this clearly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants