LeaVerou / LeaVerou/xtensible

first_ prefix doesn't work for instance hooks like first_constructor

Open
#3 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
1
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Summary

hooks.add("first_constructor", callback) fires the callback on every new B() instead of only the first. The first_ prefix is effectively a no-op for any hook where each invocation has a different instance as context.

How it was found

The test in test/hooks-integration.js ("first_ prefix") expects ["B"] but produces ["B", "B", "B"]. This test was never truly green — it only passed because hTest 0.0.24 had a bug where array equality did prefix matching (["B"] matched ["B", "B", "B"]). That was fixed in hTest 0.0.26 (require exact length for array equality). nude-element upgraded to 0.0.26 and the test now correctly fails.

Root cause

In src/hook.js, Hook.run() checks once against a per-context key:

let context = runContext ?? addOptions.context ?? fallbackContext;
let once = options?.once ?? addOptions.once;
if (once && this.hasRun(context, callback)) {
    continue;
}

hasRun uses a WeakMap<context, WeakSet<callback>>. The context for instance hooks is this at the $hook call site (the instance):

// plugins/$hook.js
$hook(name, env, options) {
    this.constructor[hooks].run(name, env, { context: this, ...options });
}

Each new B() creates a different instance, so context is always a fresh object → hasRun never finds a prior run → callback fires every time.

Why static hooks work

first_constructor_static works correctly because for static hooks, context is the class object (e.g., B, C, D), which persists across instances. The hasRun deduplication works because the same class key appears in both the subclass traversal loop and the final hook.run() call.

Tension with first_connected

first_connected goes through the same mechanism (plugin hooks object → addPluginClass[hooks].add()Hooks.resolve strips first_, sets addOptions.once = true).

first_connected needs per-instance tracking — the same element can disconnect/reconnect, and first_connected should fire only on the first connection of each element. Per-instance hasRun is correct for that case.

So:

  • first_connected needs per-instance tracking (same instance, multiple firings)
  • first_constructor needs per-class tracking (different instances, one firing each)
  • Both use the same addOptions.oncehasRun(context, callback) path

The mechanism can't currently distinguish these cases because it doesn't know whether a hook can repeat on the same context.

Question

Should first_ gain scope awareness, or is there a simpler resolution?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/hook.js and plugins/$hook.js, then run the “first_ prefix” case in test/hooks-integration.js. Compare the per-context tracking needed by first_constructor with the behavior described for first_connected. Done means the scope decision is explicit and the integration test passes without breaking the per-instance behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.