Skip to content

chore: Add TS types for lifecycle and WebGL context events - #8145

Open
camdecoster wants to merge 4 commits into
mainfrom
cam/add-missing-events-types
Open

camdecoster wants to merge 4 commits into
mainfrom
cam/add-missing-events-types

Conversation

@camdecoster

Copy link
Copy Markdown
Contributor

Description

Add TypeScript declarations for lifecycle and WebGL context events.

Changes

  • Add new types
  • Rename test file
  • Add new TS type tests

Testing

  • Check CI
  • Locally, run npm run test-types
  • Open test/types/events.ts
  • Hover over a gd.on('plotly_...', (event) => ...) call
  • Note that the proper type appears in the tooltip
  • In the same file, type gd.on('plotly_ on a new line
  • Note that you're prompted to fill in a number of event types, including the new events

Notes

@camdecoster
camdecoster marked this pull request as ready for review October 23, 2026 18:36
claude[bot]

This comment was marked as low quality.

Comment on lines +372 to +375
export type TreemapPlotDatum = SunburstPlotDatum;

/** Point payload for `plotly_icicleclick`. Same shape as `SunburstPlotDatum`. */
export type IciclePlotDatum = SunburstPlotDatum;

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.

These types reuse SunburstPlotDatum which declares that value, id, color, and hovertext are all required.
However, at runtime a click payload does NOT always have those fields:

gd.on('plotly_treemapclick', (e) => {
    const pt = e.points[0];
    panel.textContent = `${pt.label}: ${pt.value.toLocaleString()} people`; // TypeError: pt.value is undefined
    loadDetails(pt.id); // undefined without `ids`
});

Also, the sunburst event has the same gap today, and this PR extends the gap to two more events.
In this PR, nextLevel became optional for the same reason.

Comment on lines +437 to +440
on(
event: 'plotly_legendtitleclick' | 'plotly_legendtitledoubleclick',
callback: (event: LegendTitleClickEvent) => boolean
): void;

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.

An event handler that only observes the event (i.e. does NOT return false) reports a type error:

gd.on('plotly_legendtitleclick', (e) => {
    console.log({ legend: e.legendId });
});
error TS2769: No overload matches this call.
  The last overload gave the following error.
    Argument of type '"plotly_legendtitleclick"' is not assignable to parameter of type '"plotly_afterexport" | ...

Comment on lines +268 to +276
/** Payload for `plotly_react`, emitted after a `Plotly.react` call that left the config unchanged. */
export interface PlotReactEvent {
/** User-supplied config object. */
config: Partial<Config>;
/** User-supplied data array. */
data: Data[];
/** User-supplied layout object. */
layout: Partial<Layout>;
}

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.

Small mismatch here vs what the runtime actually does:

config is not optional here, but a call without a config sends config: undefined.

gd.on('plotly_react', (e) => console.log(e.config.responsive)); // no TS error
Plotly.react(gd, data, layout);
// TypeError: Cannot read properties of undefined (reading 'responsive')

Comment on lines +326 to +328
export interface WebGLContextLostEvent {
/** The original DOM `webglcontextlost` event. */
event: WebGLContextEvent;

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.

This isn't new to this PR, but I just noticed it:

npm run test-types will not catch a broken type reference inside the declarations!

Example:

 export interface WebGLContextLostEvent {
     /** The original DOM `webglcontextlost` event. */
-    event: WebGLContextEvent;
+    event: WebGLContextEvnt;
  • npx tsc --noEmit -p test/types --skipLibCheck false fails as expected
  • npm run test-types passes

It's not obvious, but test/types/tsconfig.json is extending the root tsconfig, which sets skipLibCheck: true.

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.

2 participants