SEPURI-SAI-KRISHNA opened a new pull request, #21736:
URL: https://github.com/apache/echarts/pull/21736
## Brief Information
This pull request is in the type of:
- [x] bug fixing
- [ ] new feature
- [ ] others
### What does this PR do?
Makes the `opts` argument of `echartsInstance.getConnectedDataURL` actually
optional, as the rest of the method already assumes.
### Fixed issues
<!-- No existing issue; found while auditing the public API surface. -->
## Details
### Before: What was the problem?
Calling the documented public API with no argument always threw:
```js
chart.getConnectedDataURL();
```
```
TypeError: Cannot read properties of undefined (reading 'type')
at ECharts.getConnectedDataURL (src/core/echarts.ts:1026)
```
`opts` is declared optional and every other read of it inside the method is
written
defensively, but the very first read is not:
```js
getConnectedDataURL(opts?: {...}): string {
// ...
const isSvg = opts.type === 'svg'; // <-
throws
// ...
const dpr = (opts && opts.pixelRatio) || this.getDevicePixelRatio();
// guarded
// ...
return targetCanvas.toDataURL('image/' + (opts && opts.type ||
'png'));// guarded
// ...
return this.getDataURL(opts); // getDataURL does `opts = opts || {}`
}
```
So the method is internally inconsistent: the `opts &&` guards further down
and the
delegation to `getDataURL` (which normalizes `opts` itself) are unreachable
for a
no-argument call, because line 1026 has already thrown. The failure does not
depend
on whether the chart is connected, on the renderer, or on the environment.
### After: How does it behave after the fixing?
`opts` is normalized once at the top, mirroring `getDataURL`:
```js
opts = opts || {};
```
`getConnectedDataURL()`, `getConnectedDataURL(undefined)` and
`getConnectedDataURL({...})` all behave as documented. The existing `opts &&
...`
reads are left alone — they are now simply redundant rather than
load-bearing.
## Document Info
One of the following should be checked.
- [x] This PR doesn't relate to document changes
- [ ] The document should be updated later
- [ ] The document changes have been made in apache/echarts-doc#xxx
## Misc
### Security Checking
- [ ] This PR uses security-sensitive Web APIs.
### ZRender Changes
- [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).
### Related test cases or examples to use the new APIs
Added `test/ut/spec/api/getConnectedDataURL.test.ts`: no argument, explicit
`undefined`, and an explicit options object. The first two fail on `master`;
the
third passes before and after and is kept as a control.
`npm run test`, `npx tsc --noEmit` and `eslint` on the changed file all pass.
### Merging options
- [x] Please squash the commits into a single one when merging.
### Other information
This is a different problem from #19278, which is about the SVG branch of
the same
method; this PR does not address that issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
