Fix ToImgopts/DownloadImgopts type definitions - #8058
Closed
hikmetba-bit wants to merge 1 commit into
Closed
hikmetba-bit wants to merge 1 commit into
hikmetba-bit wants to merge 1 commit into
Conversation
Both interfaces required format/width/height (and DownloadImgopts also
required filename), but the runtime (src/plot_api/to_image.js,
src/snapshot/download.js) defaults every one of them -- toImage/
downloadImage can be called with no options at all (opts = opts || {}).
Also two gaps against the runtime's attrs object in to_image.js:
- format's value union was missing 'full-json' (documented export
format, valType enumerated values includes it, dflt: 'png').
- imageDataOnly (valType boolean, dflt: false) had no type at all.
Added setBackground to both interfaces too: to_image.js's shared attrs
object accepts it (valType 'any', same per-call meaning as the existing
top-level Config#setBackground override in this same file), but neither
interface had it typed.
Fixes plotly#8057.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
We haven't decided on an approach yet, so I'm closing this PR. You're welcome to join the discussion in the issue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #8057.
Both
ToImgoptsandDownloadImgoptsrequiredformat/width/height(andDownloadImgoptsalso requiredfilename), but the runtime defaults every one of them:src/plot_api/to_image.js:opts = opts || {}—toImage()/downloadImage()can be called with no options object at all.formatdefaults to'png',scaleto1,imageDataOnlytofalse;width/heightfall back to the graph's own layout dimensions when omitted.src/snapshot/download.js: explicitly setsopts.format = opts.format || 'png',opts.width = opts.width || null,opts.height = opts.height || nullbefore use, and derivesfilenamefrom the graph title/subtitle (falling back to'plot-image') when not given.Also found two gaps against the runtime's shared
attrsobject into_image.jswhile tracing this:format's value union was missing'full-json'(valType: 'enumerated',values: [..., 'full-json'],dflt: 'png'— an exportable format since v1.53.0, per the issue).imageDataOnly(valType: 'boolean',dflt: false) had no type at all.Added
setBackgroundto both interfaces too:to_image.js's sharedattrsobject accepts it (valType: 'any'), and it's the same per-call override already typed for the top-levelConfig#setBackgroundin this same file ('opaque' | 'transparent' | ((gd, bgColor) => void)) — reused that exact type for consistency.Testing
npm run typecheck(tsc --noEmit, project-wide): clean, both before and after this change (no existing test exercised these two interfaces).{}for both interfaces,imageDataOnly: true,format: 'full-json',setBackground: 'opaque') and typechecked it directly withtsc --strict: passes cleanly against this change.config.d.tswith exactly the errors the issue describes (TS2739missing required properties,TS2353unknown propertyimageDataOnly,TS2322'full-json'not assignable) — mutation-tested by stashing just this file.🤖 Generated with Claude Code