Optional chaining as an accidental deny

A plugin without a chains option should run on every push chain. The filter I wrote made it run on none of them:

plugins.filter((plugin) => plugin.phase === element && plugin.chains?.includes(chainName))

Older plugins have no chains property at all. Optional chaining on a missing property returns undefined, undefined is falsy, and filter keeps the element only when the callback is truthy. So every plugin that predated the option was excluded from every chain, and the chain builder logged nothing about it. The plugin loaded, the startup message said so, and then it never executed.

I was adding chains to GitProxy’s push plugins so a scanner could opt out of tag pushes. The optional chaining looked like the polite way to tolerate plugins that had not been updated. It did the opposite.

What ?. actually returns

Optional chaining short-circuits to undefined when the base is null or undefined. It does not default to a reasonable value. It does not mean “skip this clause”. In a boolean position, undefined is just false.

Walk it through:

const plugin = { phase: 'AFTER_DIFF' }; // no chains, like a 2.1-era plugin
const chainName = 'branch';

plugin.chains?.includes(chainName);
// chains is undefined, so the expression is undefined
Boolean(undefined); // false

The plugin is dropped. A plugin that does set chains: ['branch', 'tag'] passes. The feature that was meant to be backwards compatible was only compatible with plugins written against the new API.

This is the same silent-drop family as a missing phase matching no marker, which I wrote up in how to write a GitProxy plugin. Both look like the plugin is broken. Both are the filter deciding the plugin does not belong.

Default first, then ask

The default has to be applied before the predicate, not inside it:

const forChain = (plugins: readonly PushActionPlugin[], name: PushChainName) =>
  plugins.filter((p) => (p.chains ?? ['branch', 'tag']).includes(name));

?? is the operator that means “if this is nullish, use that”. Optional chaining is the operator that means “if this is nullish, give up and return undefined”. They are easy to reach for interchangeably and they are not interchangeable in a filter.

Two small things fall out of putting the default in forChain rather than on the plugin class. First, a plugin compiled against an older GitProxy copy will never have run your constructor, so this.chains = options.chains ?? ['branch', 'tag'] in the constructor does not protect you. The resolver at the boundary is the load-bearing code; I argued that at more length in the plugin-system design post. Second, buildChain does not need to know about chains at all. It receives the plugins that already belong to this chain, and pull plugins never mention chains because pull has only one chain.

A lint that would have caught it

Any optional-chaining expression used directly as a boolean, especially inside filter, find, or if, is worth a second look. These are the same bug in different clothes:

if (obj.opt?.enabled) { /* skipped when opt is missing, even if you meant "default on" */ }
items.filter((x) => x.tags?.includes('prod'));

If missing should mean “include”, you want ?? default. If missing should mean “exclude”, optional chaining is correct and worth a comment, because the next reader will assume you meant the first one.

The takeaway

?. in a predicate is an accidental deny for every object that does not have the property. When you add an option to a plugin API that already has consumers in the wild, default with ?? at the point you read the option, not with ?. at the point you test it. The failure mode otherwise is the worst one a plugin system can have: the plugin loads, the logs look fine, and it never runs.