Conversation
|
@fujidana Thank you for submitting this PR! This is a live comment that I will keep updated. 1 package in this PRCode ReviewsBecause you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it. You can test the changes of this PR in the Playground. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 75624,
"author": "fujidana",
"headCommitOid": "6f2b44506ef5e4e02bd6b0824db1c81dabbd17f3",
"mergeBaseOid": "c06b1d776527b5422e8f76f3edf36982fd87e0cc",
"lastPushDate": "2026-09-28T04:21:18.000Z",
"lastActivityDate": "2026-09-28T04:21:18.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"tooManyCommits": false,
"tooManyReviews": false,
"popularityLevel": "Popular",
"pkgInfo": [
{
"name": "plotly.js",
"version": "3.0",
"kind": "edit",
"files": [
{
"path": "types/plotly.js/index.d.ts",
"kind": "definition"
},
{
"path": "types/plotly.js/test/core-tests.ts",
"kind": "test"
},
{
"path": "types/plotly.js/v2/index.d.ts",
"kind": "definition"
},
{
"path": "types/plotly.js/v2/test/core-tests.ts",
"kind": "test"
}
],
"owners": [
"chrisgervang",
"martinduparc",
"frederikaalund",
"taoqf",
"Dadstart",
"szechyjs",
"soorajpudiyadath",
"jonfreedman",
"meganrm",
"milesjos",
"skippercool",
"marnett-git",
"peterblazejewicz",
"brammitch",
"blizzardjessica",
"olegshilov",
"PabloGracia",
"jvgogh",
"jpabdou",
"mrtnbrst"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
}
],
"reviews": [],
"mainBotCommentID": 5863296047,
"ciResult": "pass"
} |
|
🔔 @chrisgervang @martinduparc @frederikaalund @taoqf @Dadstart @szechyjs @soorajpudiyadath @jonfreedman @meganrm @milesjos @SkipperCool @marnett-git @peterblazejewicz @brammitch @blizzardjessica @olegshilov @PabloGracia @jvgogh @jpabdou @mrtnbrst — please review this PR in the next few days. Be sure to explicitly select |
Please fill in this template.
pnpm test <package to test>.Select one of these and delete the others:
If changing an existing definition:
package.json.This PR fixes the following type definition problems around Plotly.js's image export features:
"full-json"literal type inToImageFormatunion type.setBackgroundproperty inToImgoptsandDownloadImgoptsinterfacesimageDataOnlyproperty inToImgoptsinterface.ToImgoptsandDownloadImgoptswrongly set as required.downloadImage()wrongly set as required.Actually
full-jsonformat option has been available since plotly.js v1.53.0 released 5 years ago (see also plotly/plotly.js#4593) but until recently it was not documented (see plotly/graphing-library-docs#471). Documentation aboutimageDataOnlywas also updated then but it seems the feature has been also available from old versions. Now the updated documentation is available in the official Function Reference and Configuration Options documentation pages.I checked
full-jsonandimageDataOnlychange the exported data as expected on both Plotly.js v2.25.2 and v3.7.0. Also, I confirmed that the properties inToImgoptsandDownloadImgoptsare actually optional (without providing them, no error is thrown) on both versions. I can't find documentation aboutsetBackgroundand don't know when it was added. Anyway, type definitions of this property will not be harmful for existing code since it is optional.Plotly.js v4 started to provide the type definitions by themselves. Reflecting the doc updates mentioned above, theier type definition was also updated (plotly/plotly.js#8066). What this PR updates are semantically very close to what their PR does. Plotly.js v4 currently only bundles the types with the main package. Therefore, for users of a partial package such as
plotly-basic-dist.minthe type definition here is still important. So is for version 3 users, of course.In the process of fixing the problems listed above, the following code changes have been made:
SetBackgroundandToImageFormattypes andToImageButtonOptionsinterface, instead of directly defining them in fields of other types/interfaces.ToImgoptsandDownloadImgoptsfromToImageButtonOptions, instead of redundantly defining their properties.