Skip to content

fix: make internally-invoked built-in transforms overridable#97

Merged
timfish merged 3 commits into
mainfrom
feat/allow-overiding-all-builtin-transforms
Jul 16, 2026
Merged

fix: make internally-invoked built-in transforms overridable#97
timfish merged 3 commits into
mainfrom
feat/allow-overiding-all-builtin-transforms

Conversation

@timfish

@timfish timfish commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Built-in transforms called each other by direct reference on the module object (e.g. tracingChannelDeclarationtracingChannelImport), so custom transforms registered via addTransform were bypassed on those internal calls.

Internal calls now dispatch through a merged transforms map (built-ins plus registered overrides) carried on state.transforms, so overriding any built-in transform including tracingChannelImport and tracingChannelDeclaration takes effect everywhere.

Moving the transforms to the state also allows transforms to chain other transforms.

Why might anyone want to override these?

Orchestrion currently only handles the publishing side of the diagnostics channel. It would be great to conditionally initialise the subscriber side too. This change would allow us to override tracingChannelImport to inject our initialisation code.

@timfish
timfish requested review from a team, bizob2828, jsumners-nr and rochdev and removed request for rochdev July 15, 2026 19:36
Comment thread lib/transforms.js
* into the program body, also ensuring the import is present.
*
* @param {{ channelName: string, module: { name: string }, dcModule: string, sourceType: 'module'|'script' }} state
* @param {{ channelName: string, module: { name: string }, dcModule: string, sourceType: 'module'|'script', transforms: Record<string, Function> }} state

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.

(not the change being made here, I realize, but the same line) shouldn't this be moduleType? Doesn't seem to match the code it's about.

Comment thread index.d.ts Outdated
* aliases resolved to local names) plus runtime fields added by the
* transformer for the current file.
*/
export type TransformState = InstrumentationConfig & {

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.

Technically, anything can be added to the state. Would that be a problem if a user of the library is also using TypeScript?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've improved the types to:

export type KnownState = InstrumentationConfig & {
    dcModule: string;
    moduleType: ModuleType;
    moduleVersion: string;
    operator: string;
    transforms: Record<string, CustomTransform>;
    functionIndex?: number;
};

export type CustomTransform<ExtraState = Record<string, unknown>> = (state: KnownState & ExtraState, node: Node, parent: Node, ancestry: Node[]) => void;

This means that downstream users can:

  • Discover and access all the known properties
  • Set/get any other properties freely (due to Record<string, unknown> default generic type)
  • Override the generic type to make accessing of their custom state more type-safe

@timfish
timfish merged commit deec36a into main Jul 16, 2026
7 checks passed
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.

3 participants