fix(dashboards): skip tile refetch when discarding a layout-only edit - #68476
Draft
posthog[bot] wants to merge 1 commit into
Draft
fix(dashboards): skip tile refetch when discarding a layout-only edit#68476posthog[bot] wants to merge 1 commit into
posthog[bot] wants to merge 1 commit into
Conversation
Discarding dashboard edits (Cancel) always fired a full refreshDashboardItems that refetched every insight, even when the only change was tile positions. That tears the grid down and reloads it for no data change, leaving the content area blank for many seconds before everything pops back in. The saved layout is already restored by the dashboard reducer, so a refetch is only needed when a filter/variable preview actually changed tile data during editing. Any such preview also writes the new state to the URL, so we compare the current URL filters/variables against the snapshot taken at edit-mode entry and only refetch when they differ. Generated-By: PostHog Code Task-Id: dd56fbd3-541f-402c-95b5-cbfc60ab1bee
Contributor
🤖 CI report✅ Bundle size — 🟢 -17.21 MiB (-21.0%)Uncompressed size of every built Total: 64.66 MiB · 🟢 -17.21 MiB (-21.0%)
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report ✅ Eager graph — within budgetHow much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy
🟢 Largest files eagerly shipped from
|
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 281.3 KiB | ../node_modules/.pnpm/posthog-js@1.406.2/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 236.0 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 224.7 KiB | ../node_modules/.pnpm/posthog-js@1.406.2/node_modules/posthog-js/dist/module.js |
| 167.1 KiB | src/queries/validators.js |
| 154.3 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 105.8 KiB | src/lib/api.ts |
| 94.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Dist folder size — 🟢 -136.14 MiB (-9.1%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1357.18 MiB · 🟢 -136.14 MiB (-9.1%)
✅ toolbar-size — eager 2.18 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.18 MiB · 17 files | (no base measurement) | ████░░░░░░ 38.1% of 5.72 MiB |
| Deferred (lazy) | 2.07 MiB · 33 files | (no base measurement) | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | (no base measurement) | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 713.8 KiB | dist/toolbar/toolbar-app-HY7HJI4V.css |
| 543.6 KiB | dist/toolbar/chunk-chunk-UG3THN3N.js |
| 484.2 KiB | dist/toolbar/chunk-chunk-QS5AHYGW.js |
| 133.6 KiB | dist/toolbar/chunk-chunk-MCXISDMN.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-T5KY5WYR.js |
| 71.0 KiB | dist/toolbar/toolbar-app-UDDFB6JG.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-27JL52RE.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-XVKSNBZ7.js |
| 20.9 KiB | dist/toolbar/chunk-chunk-CS7W2KTV.js |
| 12.2 KiB | dist/toolbar/chunk-chunk-PIK3PADE.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
Contributor
Author
|
Heads up on the red Frontend Tests Pass check: this is a CI selective-mode timeout, not a test failure. On a draft PR, frontend CI runs a single un-sharded Jest job ( Every selected suite that did run passed, including the changed Nothing to fix in the diff here. Flagging so a maintainer can mark it ready for review (or apply the frontend-CI label) to get the sharded run rather than chasing a code issue that isn't there. |
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
Contributor
Author
|
👋 Visual changes detected for this PR. Review and approve in PostHog Visual Review If these changes are unexpected, they may be caused by a flaky test or a broken snapshot on master. Don't approve — rerun the job or wait for a fix. |
Contributor
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
2 tasks
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.
Problem
Rearranging tiles on a dashboard and then clicking Cancel blanks the whole content area for many seconds before every panel suddenly reappears. Dashboard layout editing is a core, high-traffic flow, so this degrades a common interaction.
The discard/Cancel path (
setDashboardModewithDashboardHeaderDiscardChanges) always fired a fullrefreshDashboardItems, refetching every insight on the dashboard, even when the only thing that changed was tile positions. The saved layout is already restored by the dashboard reducer, so that refetch does no useful work for a layout-only edit. It just tears the grid down into loading/blank states and rebuilds it once all the blocking fetches complete.Changes
On discard, only refetch when a filter or variable preview actually changed tile data during editing. Any preview that changes data also writes the new filters/variables to the URL, so I compare the current URL state against the snapshot captured at edit-mode entry. If they match, the edit was layout-only and we skip the refetch entirely, so the grid stays put and the layout just snaps back. If a filter or variable was previewed, the refetch still runs so the reverted data loads.
I deliberately did not touch the per-tile blanking that happens during an active drag or resize. That comes from the intentional
isResizingviz unmount (a perf tradeoff that avoids redrawing the canvas every frame) and the offscreen-tile virtualization behind the dashboard item rendering flag. Reverting those risks a real perf regression, and they are separate from the full-refetch-on-Cancel behavior that caused the long blank.How did you test this code?
Automated, at the kea logic layer (
dashboardLogic.test.ts). I could not access the original session recordings (they sit in customer projects outside my tooling scope) or stand up the full dashboard stack to record the flow, so I did not manually reproduce the visual state. The behavioral change is verified deterministically instead: the refetch action is the sole trigger of the tile teardown on the layout-only Cancel path, so asserting it is not dispatched directly locks in the fix.Two new tests:
discarding a layout-only edit does not refetch tile data— enters layout edit mode, moves a tile, discards, and assertsrefreshDashboardItemsis never dispatched while the saved layout is still restored. Catches a regression where a layout-only Cancel refetches the whole grid.discarding after a filter preview still refetches tile data— previews a global date change (which writes to the URL) then discards, assertingrefreshDashboardItemsstill fires. Guards against over-correcting and leaving stale previewed data on screen.Ran the full
dashboardLogic.test.tssuite (77 passed, 1 pre-existing skip) plus lint and format on the changed files. TypeScript check is clean for these two files (the unrelated errors in the run are missing kea typegen artifacts that CI regenerates).Automatic notifications
Docs update
No docs change needed. This is an internal behavior fix with no user-facing API or documented workflow change.
🤖 Agent context
Autonomy: Fully autonomous
Authored by Claude (Opus 4.8) via PostHog Code, from an inbox report flagging dashboard tiles blanking during edit and the grid vanishing after Cancel. I traced the Cancel symptom to the unconditional
refreshDashboardItemsin thesetDashboardModediscard branch ofdashboardLogic.tsx, confirmed via the reducers and selectors that previewed filters/variables always land in the URL, and used the existing edit-mode URL snapshot to distinguish layout-only discards from filter/variable-preview discards.I considered the report's other suggestions (dropping the
!hasResults && loadingQueuedloading branch inInsightCard, and guarding the empty-state gate inDashboard.tsx) and rejected them. The!hasResults && loadingQueuedbranch only shows a loading state when a tile has no results at all, so it is already stale-while-revalidate for loaded tiles, and dropping it would regress initial load. TheDashboard.tsxempty-state gate is not reached on the discard path, since discard never nulls the dashboard. Fixing either would be adjacent to the actual observed symptom. Invoked the/writing-testsskill before adding the two logic tests.Created with PostHog Code from an inbox report.