Losing this by passing a method as a callback

Take a method off an object and pass it as a callback, and this inside that method is no longer the object. It is undefined in strict mode, which TypeScript and "use strict" both are. I inherited this in GitProxy’s old plugin insertion:

branchPushChain.splice(0, 0, pluginObj.exec);

pluginObj.exec is a function. Once it is sitting in the chain array, the call site looks like fn(req, action). There is no pluginObj on the left of that call, so there is no receiver. Any plugin that stored state on the instance and read it from exec blew up the first time it ran, which is awkward, because GitProxy’s own docs tell authors to subclass because they have state.

this is set by the call site, not by where the function was defined

MDN’s page on this is the whole rule: in a function that is not an arrow, this depends on how the function is called. obj.method() sets this to obj. const fn = obj.method; fn() does not. Extracting the reference is enough. The chain array is just a delayed version of that extraction.

This is not unique to plugins. It is the same bug as passing this.handleClick to addEventListener without binding it, or passing obj.process to Promise.then. The plugin version is harder to spot because the extraction and the call are in different files, several ticks apart.

Three ways to keep the receiver

An arrow function as a class field closes over this at creation, so extracting it does not matter:

class RunOnPushPlugin extends PushActionPlugin {
  #seen = 0;
  exec = async (req: Request, action: Action) => {
    this.#seen += 1;
    return action;
  };
}

That is the form GitProxy already recommends for subclasses with state, and it is why most sample plugins survived the old insertion. A regular method would not:

class RunOnPushPlugin extends PushActionPlugin {
  seen = 0;
  async exec(req: Request, action: Action) {
    this.seen += 1; // TypeError: Cannot read properties of undefined
    return action;
  }
}

Function.prototype.bind produces a wrapper with this fixed:

branchPushChain.splice(0, 0, pluginObj.exec.bind(pluginObj));

That would have been a one-line fix to the old insertion. It still mutates the author’s function’s calling convention only for this one reference, which is fine, but it does not attach displayName or isCollectible.

The version GitProxy ended up with does both, by wrapping at chain-build time:

const toPluginExec = (plugin: ActionPlugin): ProcessorExec =>
  Object.assign((req: Request, action: Action) => plugin.exec(req, action), {
    displayName: plugin.displayName ?? `${plugin.constructor.name}.exec`,
    isCollectible: plugin.isCollectible ?? false,
  });

plugin.exec(req, action) is a method call, so this is plugin whether exec is an arrow field or a prototype method. The wrapper is a new function object, so two plugins that share one exec reference can still have different displayName and isCollectible. Nothing is stapled onto the author’s function, which is what readonly on ProcessorExec was asking for in the first place.

Why the old code looked fine in tests

The fixtures passed a bare { exec } object, and those exec functions never touched this. The built-in processors are standalone functions, not methods, so they never needed a receiver either. The only code path that could have failed was a stateful subclass, which is exactly the path the docs encourage and exactly the path unit tests did not exercise. A single test that constructs new PushActionPlugin as a real subclass with instance fields, then runs it through getChain, would have caught this.

The takeaway

If you store a function in a list to call later, and that function is a method, the list has to store the receiver too. Bind it, wrap it, or define it as an arrow field. Pulling obj.method out and calling it later is how this becomes undefined, and it will only show up in the plugins that have state, which are the ones whose authors followed the docs. The wrapper above is part of the chain builder in the plugin-system design post.