Update Plotly.js to 4.0.0-rc.0 - #19377
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19377Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19377" |
There was a problem hiding this comment.
Pull request overview
Updates the dashboard metrics chart to use Plotly.js Basic 4.0.0-rc.0.
Changes:
- Updates the Plotly.js bundle import from 2.35.2 to 4.0.0-rc.0.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Adam Ratzman (adamint)
left a comment
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
I did not find a code issue here. I verified the vendored Plotly bundle is byte-for-byte the official plotly-basic-4.0.0-rc.0 asset. I was not able to exercise the browser rendering path in this environment, so leaving this as a comment rather than an approval.
PR Testing ReportPR Information
Artifact Version Verification
Chain of custody — the tested bytes are provably the PR's bytes:
Changes AnalyzedFiles Changed
Change Categories
Why extra rigor was warrantedCI is fully green (363 success / 4 skipped), but no existing test executes this bundle:
Green CI therefore provides no signal on this change. All testing below was purpose-built. Test MethodBecause Two harnesses were used:
Test Scenarios ExecutedScenario 1: Bundle identity & API surfaceObjective: Verify the shipped bundle is the intended upstream artifact and exposes every API
Scenario 2: Chart renders with real trace dataObjective: Verify a 3-series stacked area chart + exemplar trace renders. 3 SVG elements, 4 traces bound, 3 Evidence: Scenario 3: Exemplar hover → pointer cursorObjective: Verify Real mouse hover over an exemplar marker sets
Scenario 4: Exemplar click →
|
| Check | 2.35.2 | 4.0.0-rc.0 | Verdict |
|---|---|---|---|
| SVG rendered | 3 | 3 | same |
| Exemplar points | 4 | 4 | same |
| Stack fills | 3 | 3 | same |
paper_bgcolor applied |
#f7f7f7 |
#f7f7f7 |
same |
| Exemplar marker color | #6f3dff |
#6f3dff |
same |
| Hover → cursor | pointer |
pointer |
same |
Click → ViewSpan args |
["trace-4","span-4"] |
["trace-4","span-4"] |
same |
| Empty dataset | throws TypeError |
throws TypeError |
same (pre-existing) |
| Console/page errors | 0 | 0 | same |
No rendering or behavioral regression was found in the upgrade.
Findings
🟠 The v4 cloud-upload default flip is real, and only one line suppresses it
Probing the libraries' own config defaults (no displayModeBar override):
| Config default | 2.35.2 | 4.0.0-rc.0 |
|---|---|---|
showSendToCloud |
false |
true |
plotlyServerURL |
"" |
https://cloud.plotly.com/newchart |
| Cloud/share button in modebar | absent | present |
Rendered modebar buttons:
- 2.35.2:
Download plot as a png, Zoom, Pan, Box Select, Lasso Select, Zoom in, Zoom out, Autoscale, Reset axes - 4.0.0-rc.0:
Download plot as a PNG,Share Chart, Zoom, Pan, Box Select, Lasso Select, Zoom in, Zoom out, Autoscale, Reset axes
Today this is not exploitable — app-metrics.js:83 passes displayModeBar: false, and that is the
only Plotly.newPlot call site, so the modebar is destroyed before the button exists (verified: 0 modebar,
0 share buttons, 0 outbound requests).
But: with displayModeBar: false, gd._context.showSendToCloud is still true on v4 (vs false on 2.35.2).
The only thing standing between a user's telemetry and cloud.plotly.com is that single boolean.
Anyone enabling the modebar later — a reasonable, innocuous-looking change — silently ships a
"Share Chart" button that window.opens plotly's cloud and postMessages the full chart JSON.
CSP does not help: default-src 'self' (BrowserSecurityHeadersMiddleware.cs:57) governs neither
window.open nor postMessage, and it is not applied in Development at all.
Recommendation (cheap, permanent): add to the config object in app-metrics.js:
showSendToCloud: false,
plotlyServerURL: ''🟠 Shipping a pre-release into production
npm dist-tags at time of testing: latest = 3.7.0 (stable), rc = 4.0.0-rc.0. 4.0.0 final is unpublished.
This PR jumps 2.35.2 → a 4.x release candidate, skipping the entire stable 3.x line. Worth confirming this
is deliberate and that there's a tracking item to move to 4.0.0 final once released.
🟡 No automated test covers the chart JS
This upgrade could have broken exemplar hover, click-to-trace, theming, or locale formatting and every
existing test would still pass. Consider adding a Playwright case that actually renders a metrics chart.
🔵 Minor / pre-existing
fixTraceLineRenderingshould null-check.scatterlayer(pre-existing; reproduces on 2.35.2)..config/PoliCheckExclusions.xml:3still referencesplotly-2.32.0.min.js(already stale onmain).THIRD-PARTY-NOTICES.TXT:16says "Copyright (c) 2021 Plotly, Inc"; the new bundle header says "Copyright 2012-2026".
Summary
| Scenario | Status | Notes |
|---|---|---|
| 1. Bundle identity & API surface | ✅ Passed | Upstream hash match; old bundle 404 |
| 2. Chart renders with real data | ✅ Passed | Stacks, exemplars, legend all correct |
| 3. Exemplar hover → cursor | ✅ Passed | A/B identical to 2.35.2 |
4. Exemplar click → ViewSpan |
✅ Passed | traceData round-trip intact |
5. updateChart path |
✅ Passed | Data updates, exemplars survive |
| 6. FluentUI theme colors | ✅ Passed | New color lib accepts Aspire tokens |
| 7. Locale registration | ✅ Passed | Server time format applied |
| 8. No cloud-upload surface | ✅ Passed | 0 requests to plotly.com — see Findings |
| 9. Empty dataset | Identical failure on 2.35.2 | |
| 10. Re-init / hide-show / resize | ✅ Passed | No leaks, no throws |
| 11. JS error surface | ✅ Passed | 0 console / 0 page errors |
Automated checks: 25 passed, 0 failed, 1 pre-existing (verified identical on 2.35.2).
Overall Result
✅ PR FUNCTIONALLY VERIFIED — no regression found
The bundle is authentic, the upgrade is behaviorally identical to 2.35.2 across every path
app-metrics.js exercises, and no telemetry leaves the browser.
Recommendations
- Add
showSendToCloud: falseandplotlyServerURL: ''to the Plotly config — 2 lines, removes a
silent data-egress footgun that the v4 defaults introduced. - Confirm the pre-release is intentional and track moving to
4.0.0final. - (Optional) Add a Playwright test that renders a metrics chart, so the next bundle bump has real coverage.
- (Optional, pre-existing) Null-check
.scatterlayerinfixTraceLineRendering; refresh the stale
PoliCheck exclusion and third-party notice copyright year.
Plotly 4.x flipped the showSendToCloud config default from false to true and set plotlyServerURL to https://cloud.plotly.com/newchart. With the modebar displayed, that renders a "Share Chart" button which window.open()s Plotly's cloud service and postMessage()s the full chart JSON to it. Verified against both bundles with the same app-metrics.js: config default 2.35.2 4.0.0-rc.0 showSendToCloud false true plotlyServerURL "" https://cloud.plotly.com/newchart share button absent "Share Chart" The dashboard passes displayModeBar: false, so the button is not reachable today. But showSendToCloud remained true on the graph context, meaning a later change that enables the modebar would silently ship a control that uploads user telemetry to a third party. CSP does not mitigate it: neither window.open nor postMessage is covered by default-src 'self', and the policy is not applied in Development at all. Setting both values explicitly makes the guarantee independent of the modebar setting. Confirmed the "Share Chart" button no longer appears even when displayModeBar is forced to true. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
James Newton-King (@JamesNK) I pushed one commit to this branch — What it does: adds Why: Plotly 4.x flipped these defaults. Verified empirically by running the same
Rendered modebar buttons:
This was not a live bug. Verified after the change: Happy to drop it if you'd prefer to keep the diff purely mechanical and handle this separately. |
Mitch Denny (mitchdenny)
left a comment
Mitch Denny (mitchdenny)
left a comment
There was a problem hiding this comment.
Approving. The bundle is authentic and the upgrade is behaviorally identical to 2.35.2 across every path the dashboard exercises.
Supply chain verified. SHA-256 of the vendored file matches upstream npm plotly.js@4.0.0-rc.0/dist/plotly-basic.min.js byte-for-byte (e19a9643a8a9…e257dc). Traced end to end: dogfood CLI 13.6.0-pr.19377.g37fc8fde → Aspire.Dashboard.Sdk nupkg ships only the new bundle → served 200, old one 404s → window.Plotly.version === 4.0.0-rc.0 at runtime.
Behavior verified, not assumed. Nothing in CI executes this bundle — PlotlyChartTests asserts against bUnit's mocked JSInterop, and the only Playwright /metrics case never renders a chart. So I ran the real app-metrics.js against the real shipped artifact, and separately A/B'd the same, unmodified module against both bundles. Rendering, stacked areas, exemplar markers, hover → pointer cursor, click → ViewSpan(traceId, spanId), the traceData round-trip, Plotly.update, locale registration, and FluentUI theme colors through v4's new color library are all identical to 2.35.2. Zero console errors, zero page errors, zero external requests. 25 passed, 0 failed.
The one crash I found (empty dataset → TypeError in fixTraceLineRendering, which doesn't null-check .scatterlayer) reproduces identically on 2.35.2, so it's pre-existing and not a blocker for this PR.
Disclosure: I pushed 2b974f5 to this branch myself, so my approval covers a commit I authored. It sets showSendToCloud: false and plotlyServerURL: '' — v4 flipped those defaults and renders a Share Chart button that uploads chart JSON to cloud.plotly.com. Not reachable today given displayModeBar: false, but the flag stayed true on the graph context. James Newton-King (@JamesNK), please revert it if you'd rather keep this diff purely mechanical — the approval stands either way.
One thing worth confirming before merge: this ships a pre-release. npm latest is 3.7.0; 4.0.0 final is unpublished. The PR jumps 2.35.2 → 4.0.0-rc.0, skipping the stable 3.x line entirely. Approving on the assumption that's deliberate — a tracking item to move to 4.0.0 final once it ships would be good.
Optional follow-ups, none blocking: add a Playwright test that actually renders a metrics chart so the next bundle bump has real coverage; null-check .scatterlayer; refresh .config/PoliCheckExclusions.xml:3 (still references plotly-2.32.0.min.js, already stale on main) and the THIRD-PARTY-NOTICES.TXT:16 copyright year.
| @@ -1,4 +1,4 @@ | |||
| import './plotly-basic-2.35.2.min.js' | |||
| import './plotly-basic-4.0.0-rc.0.min.js' | |||
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Description
Updates the Plotly.js basic bundle used by the dashboard metrics graph from 2.35.2 to 4.0.0-rc.0. This keeps the metrics visualization on the upcoming Plotly.js major release while retaining the smaller basic distribution used for scatter traces.
Validation:
dotnet build src/Aspire.Dashboard/Aspire.Dashboard.csproj --no-restorePlotlyChartTests(4 passed)Plotly.versionreporting4.0.0-rc.0Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?