Raman-Katiyar opened a new pull request, #46081:
URL: https://github.com/apache/superset/pull/46081
### SUMMARY
Fixes #45885.
Downloading a dashboard as an image freezes and can crash the Chrome tab on
dashboards with large pivot tables (a regression vs 6.1.0, which completed the
same export in 20–30 s).
Root cause (analysis by @FrancescoCastaldi in #45885, confirmed by the
measurements below): `dom-to-image-more`, used by `downloadAsImage`, copies
**every** computed style property onto **every** cloned node. Since the antd
CSS-variable theming change, each element inherits ~440 `--*` custom
properties, none of them default-valued, so the library inlines all of them
onto every clone instead of skipping them the way it skips default-valued
properties. On the reporter's ~11,000-cell pivot table that is more than 4M
`setProperty` calls during a single capture.
This change passes the library's `filterStyles` option in
`src/utils/downloadAsImage.tsx` so CSS custom properties (`--*`) are excluded
from the style copy, in both dom-to-image call paths in that file (the general
enhanced-clone path used for dashboards/charts and the ag-grid path). Excluding
them does not change the rendered output:
- custom properties are inherited from the capture root, which keeps its own
styles, so any `var(--x)` in a copied real property still resolves against the
same values;
- every real property is still copied as its already-resolved computed value.
Scope note: this PR covers the **Download as image** path. **Export to PDF**
(`src/utils/downloadAsPdf.ts`) uses `dom-to-pdf`, which exposes no equivalent
style-filtering option; replacing that library is the open question raised in
the issue thread, so it is intentionally not part of this change.
#### Reproduction / measurements
I reproduced the mechanism in a real Chrome (157) using the exact
`dom-to-image-more` 3.11.0 build shipped in `superset-frontend`, driving it
through the same clone-then-capture flow as `downloadAsImage.tsx` (off-screen
attached clone, `toPng`), with a table under a `:root` carrying 440 `--ant-*`
custom properties. A sample cell computes **920 style properties, 440 of them
custom** — the same numbers reported in the issue (480 → 920 per cell from
6.1.0 to 7.0.0rc2). Counting `CSSStyleDeclaration.setProperty` calls during
capture:
| Table size | Before (no filter) | After (`filterStyles`) | PNG output |
|---|---|---|---|
| 3,000 cells | 1,442,462 calls, 12.4 s | 108,382 calls (13.3× fewer), 4.2 s
| identical (data-URL length 1,220,998 both runs) |
| 11,000 cells (reporter's scale) | 5,286,622 calls, 66.0 s, page repeatedly
unresponsive | 397,342 calls (13.3× fewer), 15.3 s, responsive throughout |
identical (data-URL length 4,583,142 both runs) |
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — no visual change. The exported image is byte-identical in
the reproduction above (same PNG data-URL length before and after, at both
scales); only the number of style copies performed during capture changes.
### TESTING INSTRUCTIONS
Automated:
- `npm run test -- src/utils/downloadAsImage.test.ts`
- On unmodified master the two new regression tests fail
(`options.filterStyles` is `undefined` on both dom-to-image paths); the 43
pre-existing tests pass.
- With this change all **45 tests pass**, including the two new tests,
which assert for both call paths that a `filterStyles` function is passed to
dom-to-image-more, that it returns `false` for custom properties
(`--ant-color-primary`, `--colorBgContainer`) and `true` for real properties
(`color`, `font-size`, `background-color`).
- Lint: `oxlint` reports 0 errors on the touched files (the only warnings
are two pre-existing `no-console` warnings in untouched code); the pre-commit
hooks that can run in this environment (oxfmt, oxlint, custom rules, stylelint)
pass.
- Type-check: a full-repo `tsc --noEmit` in this environment produces an
identical (normalised) error set on master and on this branch — no new errors
in the touched files (the touched test file has none on either;
`downloadAsImage.tsx` shows only the same pre-existing environment-artifact
errors as master). A fresh checkout here cannot complete the check cleanly
because the workspace packages are not built locally (`TS6305`); CI builds them
and type-checks properly.
Manual:
1. `superset load-examples`, build a dashboard with a Pivot Table on
`birth_names` (rows `name`, columns `ds` with Year grain, metric `SUM(num)`,
row limit 10000 — roughly 11,000 cells) plus a Table chart, as in the issue.
2. Download → Download as image: the export completes without the tab
freezing or crashing, and the downloaded image renders the dashboard unchanged.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #45885
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Fixes #45885
--
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]
