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
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
Contributor
Author
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. Good idea! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| /** | ||
| * 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; | ||
|
|
||
| // A range value can be a number or a date string | ||
| const numberRangeStart: number | undefined = event['xaxis.range[0]']; | ||
| const dateRangeStart: 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,83 @@ | ||
| /** | ||
| * Consumer-side checks for the `relayout`, `restyle` and `update` types. | ||
| * | ||
| * Nothing here runs. `npm run test-types` compiles this file against the public entry point. | ||
| * 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' }); | ||
|
|
||
| // A wrapper that reads the parameter types must see both call forms | ||
| type RelayoutArg = Parameters<typeof Plotly.relayout>[1]; | ||
| export const relayoutObject: RelayoutArg = { autosize: true }; | ||
| export const relayoutPath: RelayoutArg = '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' }); | ||
|
|
||
| type RestyleArg = Parameters<typeof Plotly.restyle>[1]; | ||
| export const restyleObject: RestyleArg = { visible: false }; | ||
| export const restylePath: RestyleArg = '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?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.
Just to make it obvious what the failure is.