Repository navigation
fix(types): Accept attribute paths in plotly_relayout, relayout, restyle, and update #8126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| - Update TypeScript types so that `Plotly.relayout`, `Plotly.restyle`, `Plotly.update`, and the `plotly_relayout` event accept attribute paths such as `'xaxis.autorange'` [[#8126](https://github.com/plotly/plotly.js/pull/8126)] | ||
| - Add the `LayoutUpdate` and `DataUpdate` types for these update objects | ||
| - Add the `(root, attribute, value)` call form to the types of `Plotly.relayout` and `Plotly.restyle` |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The overloads you add here are good, but can still report "errors" on perfectly valid code. Example: import * as Plotly from 'plotly.js';
type RelayoutArg = Parameters<typeof Plotly.relayout>[1];
const wrapped: RelayoutArg = { autosize: true };There is a red squiggle as typescript evaluates against the "last" overload and completely misses the other valid overload. export function relayout(root: Root, astr: string | LayoutUpdate, val?: any): Promise<PlotlyHTMLElement>;The same comment applies to the |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import type { | |
| } from '../generated/schema'; | ||
| import type { Datum } from '../lib/common'; | ||
| import type { Config } from './config'; | ||
| import type { LayoutUpdate } from './layout'; | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Point / datum types in events | ||
|
|
@@ -149,10 +150,11 @@ export interface PlotRestyleEventUpdate { | |
| export type PlotRestyleEvent = [PlotRestyleEventUpdate, number[]]; | ||
|
|
||
| /** | ||
| * Payload for `plotly_relayout`. Carries a `Partial<Layout>` plus a few | ||
| * common bracketed axis-range/autorange keys that flatten nested updates. | ||
| * Payload for `plotly_relayout`. Carries a `LayoutUpdate`, so any attribute | ||
| * path is a valid key, plus typed values for a few common axis-range and | ||
| * autorange keys that flatten nested updates. | ||
| */ | ||
| export interface PlotRelayoutEvent extends Partial<Layout> { | ||
| export interface PlotRelayoutEvent extends LayoutUpdate { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This type contains 6 hand-crafted keys ( What do you think about moving the hand-typed keys here into |
||
| /** New x-axis range minimum, when the user changed the range. */ | ||
| 'xaxis.range[0]'?: number; | ||
| /** New x-axis range maximum, when the user changed the range. */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| /** | ||
| * Consumer-side checks for the `plotly_relayout` event payload. | ||
| * | ||
| * Nothing here runs. See `update_api.ts`. | ||
| */ | ||
|
|
||
| import type { PlotRelayoutEvent } from '../../lib/index'; | ||
|
|
||
| declare const event: PlotRelayoutEvent; | ||
|
|
||
| // Typed keys keep their value types | ||
| const rangeStart: number | undefined = event['xaxis.range[0]']; | ||
| const autorange: boolean | undefined = event['xaxis.autorange']; | ||
|
|
||
| // @ts-expect-error the typed range key holds a number, not a string | ||
| const rangeStartAsString: string | undefined = event['xaxis.range[0]']; | ||
|
|
||
| // Any other attribute path is readable | ||
| const secondAxisStart = event['xaxis2.range[0]']; | ||
| const shapeStart = event['shapes[0].x0']; | ||
|
|
||
| // Layout properties are still typed | ||
| const autosize: boolean | undefined = event.autosize; | ||
|
|
||
| // @ts-expect-error unknown top-level key | ||
| const typo = event.titl; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "extends": "../../tsconfig.json", | ||
| "include": ["./**/*.ts"], | ||
| "exclude": [] | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,78 @@ | ||||||
| /** | ||||||
| * Consumer-side checks for the `relayout`, `restyle` and `update` types. | ||||||
| * | ||||||
| * Nothing here runs. `npm run typecheck` compiles this file against the public entry point. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
|
||||||
| * A line marked `@ts-expect-error` must fail to compile, so the build breaks if a type becomes too permissive. | ||||||
| */ | ||||||
|
|
||||||
| import * as Plotly from '../../lib/index'; | ||||||
|
|
||||||
| declare const gd: HTMLElement; | ||||||
|
|
||||||
| // --------------------------------------------------------------------------- | ||||||
| // relayout | ||||||
| // --------------------------------------------------------------------------- | ||||||
|
|
||||||
| Plotly.relayout(gd, { 'xaxis.autorange': true }); | ||||||
| Plotly.relayout(gd, { 'xaxis.range[0]': 0, 'xaxis.range[1]': 5 }); | ||||||
| Plotly.relayout(gd, { 'xaxis.title.font.size': 14 }); | ||||||
| Plotly.relayout(gd, { 'annotations[0].text': 'hello' }); | ||||||
| Plotly.relayout(gd, { 'updatemenus[0].buttons[1].label': 'hello' }); | ||||||
| Plotly.relayout(gd, { xaxis: { autorange: true } }); | ||||||
| Plotly.relayout(gd, { autosize: true }); | ||||||
| Plotly.relayout(gd, 'xaxis.autorange', true); | ||||||
| Plotly.relayout(gd, 'annotations[0].text', 'hello'); | ||||||
|
|
||||||
| // @ts-expect-error top-level typo | ||||||
| Plotly.relayout(gd, { titl: 'abc' }); | ||||||
| // @ts-expect-error wrong value type on a known top-level key | ||||||
| Plotly.relayout(gd, { autosize: 'yes' }); | ||||||
| // @ts-expect-error the string form needs a value | ||||||
| Plotly.relayout(gd, 'xaxis.autorange'); | ||||||
|
|
||||||
| // --------------------------------------------------------------------------- | ||||||
| // restyle | ||||||
| // --------------------------------------------------------------------------- | ||||||
|
|
||||||
| Plotly.restyle(gd, { 'marker.color': 'red' }); | ||||||
| Plotly.restyle(gd, { 'marker.color': 'red' }, [0]); | ||||||
| Plotly.restyle(gd, { 'line.width': 2, 'marker.size[0]': 10 }, 1); | ||||||
| Plotly.restyle(gd, { visible: false }); | ||||||
| Plotly.restyle(gd, 'marker.color', 'red'); | ||||||
| Plotly.restyle(gd, 'marker.color', ['red', 'blue'], [0, 1]); | ||||||
|
|
||||||
| // @ts-expect-error top-level typo | ||||||
| Plotly.restyle(gd, { colr: 'red' }); | ||||||
| // @ts-expect-error wrong value type on a known top-level key | ||||||
| Plotly.restyle(gd, { visible: 'maybe' }); | ||||||
| // @ts-expect-error the string form needs a value | ||||||
| Plotly.restyle(gd, 'marker.color'); | ||||||
|
|
||||||
| // --------------------------------------------------------------------------- | ||||||
| // update | ||||||
| // --------------------------------------------------------------------------- | ||||||
|
|
||||||
| Plotly.update(gd, { 'marker.color': 'red' }, { 'xaxis.range[0]': 0 }); | ||||||
| Plotly.update(gd, { 'marker.color': 'red' }, { 'xaxis.range[0]': 0 }, [0]); | ||||||
| Plotly.update(gd, { visible: false }, { autosize: true }); | ||||||
|
|
||||||
| // @ts-expect-error top-level typo in the trace update | ||||||
| Plotly.update(gd, { colr: 'red' }, { autosize: true }); | ||||||
| // @ts-expect-error top-level typo in the layout update | ||||||
| Plotly.update(gd, { visible: false }, { titl: 'abc' }); | ||||||
|
|
||||||
| // --------------------------------------------------------------------------- | ||||||
| // APIs that take a full figure must stay strict about unknown keys | ||||||
| // --------------------------------------------------------------------------- | ||||||
|
|
||||||
| Plotly.newPlot(gd, [], { autosize: true }); | ||||||
| Plotly.validate([], { autosize: true }); | ||||||
|
|
||||||
| // @ts-expect-error a dotted path is not a valid key in a full layout | ||||||
| Plotly.newPlot(gd, [], { 'xaxis.autorange': true }); | ||||||
| // @ts-expect-error a dotted typo must not pass in a full layout | ||||||
| Plotly.newPlot(gd, [], { 'xaxis.autrange': true }); | ||||||
| // @ts-expect-error a dotted path is not a valid key when validating a layout | ||||||
| Plotly.validate([], { 'xaxis.autorange': true }); | ||||||
| // @ts-expect-error a dotted path is not a valid key in a template layout | ||||||
| const template: Plotly.Template = { layout: { 'xaxis.autorange': true } }; | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could we just add
test-typesas a second step in thetypecheckjob above? Is there a reason you want a second runner?