Skip to content

Commit be1673f

Browse files
AlemTuzlakAlemTuzlak
committed
fix(devtools): stop the SEO head watcher looping and unblock CI
The `Test` job died with a JavaScript heap OOM and the `E2e` job failed on all eight apps. Two separate causes. The head watcher looped. `createHeadChanges` observes attributes and character data across the whole `<head>` subtree, and goober rewrites a `<style>` tag there on every `css()` call — it even re-stamps the tag's `nonce` attribute each time. So an SEO analysis re-rendered, the re-render emitted CSS, the CSS mutated `<head>`, and the analysis ran again. The loop is synchronous, so no test timeout could break it and the worker ran to the 4 GB heap limit. Stylesheets carry no SEO metadata, so they are filtered out. jsdom multiplied goober's stylesheets. goober finds its single `<style id="_goober">` through `window._goober`, the global a browser creates for any element with an `id`. jsdom does not do that for `<style>`, so goober appended a new sheet on every `css()` call — about 2500 per Workbench mount, never removed. Test 1 took 0.9s and test 20 took 21s. The test setup now gives goober the global a browser would have: 2504 sheets per mount become 8, and the package's 236 tests run in 38s instead of running out of memory. That let `workbench.test.tsx` finish for the first time, which exposed nine assertions still describing the pre-polish design — the header's trailing gutter, the strip's 8px gutter, the 24px resize handle, a fixed 44px grid row, the strip unmounting when folded, and the SEO label foregrounds. Each now matches the shipped Workbench. The redesign also dropped the test hooks `@tanstack/devtools-e2e` locates the panel and header controls with, which is why every e2e app failed on `openViaTrigger()`. The header carries them again and the tab assertions read `data-tsd-selected` instead of the `active` class the old tabs used.
1 parent 6e56026 commit be1673f

8 files changed

Lines changed: 134 additions & 32 deletions

File tree

‎.changeset/tanstack-devtools-branding.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,6 @@ The Workbench now separates chrome from canvas: the header and the secondary str
1010

1111
The secondary strip gets a pull tab on its bottom edge that folds the strip away behind the header, leaving the panel height and the destination content untouched. It only appears on destinations that have a strip.
1212

13+
The SEO tab's `<head>` watcher no longer reports `<style>` tags. It observes attributes and character data across the whole `<head>` subtree, and a CSS-in-JS library rewrites a `<style>` tag there on every render — so an SEO analysis triggered a re-render, the re-render emitted CSS, and the CSS triggered another analysis. Stylesheets carry no SEO metadata, so they are filtered out.
14+
1315
Fixes along the way: the resize handle had grown to 24px and sat on top of the header, so a press aimed at a header button started a resize instead of clicking; the Marketplace settings drawer was `position: fixed` and covered the host page instead of the Workbench; the floating trigger lost its brand fill on hover and its transition was overridden away; the "New" ribbon on a plugin card overlapped the card icon; scroll gestures inside the panel chained on to the host page; and the hotkey editor showed each shortcut's description as its heading and never rendered its title.

‎e2e/apps/react-vite/tests/tabs-and-plugin.spec.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ test('settings tab becomes active when clicked', async ({ page }) => {
1515
await dt.goto()
1616
await dt.openViaTrigger()
1717
await dt.tab('settings').click()
18-
await expect(dt.tab('settings')).toHaveClass(/active/)
18+
await expect(dt.tab('settings')).toHaveAttribute('data-tsd-selected', 'true')
1919
})
2020

2121
test('plugins tab becomes active when clicked', async ({ page }) => {
@@ -24,5 +24,5 @@ test('plugins tab becomes active when clicked', async ({ page }) => {
2424
await dt.openViaTrigger()
2525
await dt.tab('settings').click()
2626
await dt.tab('plugins').click()
27-
await expect(dt.tab('plugins')).toHaveClass(/active/)
27+
await expect(dt.tab('plugins')).toHaveAttribute('data-tsd-selected', 'true')
2828
})

‎e2e/helpers/src/selectors.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ export type TabId = 'plugins' | 'seo' | 'settings'
33
export const SELECTORS = {
44
/** The trigger button is selected by its accessible name. */
55
triggerName: 'Open TanStack Devtools',
6-
mainPanel: 'tsd-main-panel',
6+
mainPanel: 'tanstack-devtools-panel',
77
resizeHandle: 'tsd-resize-handle',
88
pipButton: 'tsd-pip-button',
99
closeButton: 'tsd-close-button',

‎packages/devtools/src/components/workbench-header.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export const WorkbenchHeader = (props: {
7676
return (
7777
<button
7878
type="button"
79+
data-testid={`tsd-tab-${destination}`}
7980
data-tsd-control
8081
class={styles().workbenchNavButton}
8182
data-tsd-selected={isSelected(destination) ? 'true' : undefined}
@@ -92,6 +93,7 @@ export const WorkbenchHeader = (props: {
9293
type="button"
9394
aria-label="Settings"
9495
title="Settings"
96+
data-testid="tsd-tab-settings"
9597
data-tsd-control
9698
class={styles().workbenchActionButton}
9799
data-icon="cogs"
@@ -107,6 +109,7 @@ export const WorkbenchHeader = (props: {
107109
type="button"
108110
aria-label="Detach TanStack Devtools"
109111
title="Detach into its own window"
112+
data-testid="tsd-pip-button"
110113
data-tsd-control
111114
class={styles().workbenchActionButton}
112115
onClick={detach}
@@ -117,6 +120,7 @@ export const WorkbenchHeader = (props: {
117120
type="button"
118121
aria-label="Close TanStack Devtools"
119122
title="Close TanStack Devtools"
123+
data-testid="tsd-close-button"
120124
data-tsd-control
121125
class={styles().workbenchActionButton}
122126
onClick={props.toggleOpen}

‎packages/devtools/src/hooks/use-head-changes.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,20 @@ type UseHeadChangesOptions = {
3434
observeTitle?: boolean
3535
}
3636

37+
/**
38+
* A `<style>` tag in `<head>` never carries SEO metadata, and CSS-in-JS
39+
* libraries write into one constantly — goober alone re-stamps its `nonce`
40+
* attribute and rewrites its text on every `css()` call. Reporting those as head
41+
* changes feeds a loop: the consumer re-renders, the re-render emits CSS, the
42+
* CSS mutates `<head>`, and the observer fires again. Stylesheets are filtered
43+
* out so only real metadata reaches the consumer.
44+
*/
45+
const isStyleNode = (node: Node): boolean => {
46+
const element =
47+
node.nodeType === 3 /* Node.TEXT_NODE */ ? node.parentNode : node
48+
return (element as Element | null)?.nodeName === 'STYLE'
49+
}
50+
3751
export function createHeadChanges(
3852
onChange: (change: HeadChange, raw?: MutationRecord) => void,
3953
opts: UseHeadChangesOptions = {},
@@ -49,11 +63,14 @@ export function createHeadChanges(
4963
const headObserver = new MutationObserver((mutations) => {
5064
for (const m of mutations) {
5165
if (m.type === 'childList') {
52-
m.addedNodes.forEach((node) => onChange({ kind: 'added', node }, m))
53-
m.removedNodes.forEach((node) =>
54-
onChange({ kind: 'removed', node }, m),
55-
)
66+
m.addedNodes.forEach((node) => {
67+
if (!isStyleNode(node)) onChange({ kind: 'added', node }, m)
68+
})
69+
m.removedNodes.forEach((node) => {
70+
if (!isStyleNode(node)) onChange({ kind: 'removed', node }, m)
71+
})
5672
} else if (m.type === 'attributes') {
73+
if (isStyleNode(m.target)) continue
5774
const el = m.target as Element
5875
onChange(
5976
{

‎packages/devtools/tests/seo-workbench.test.tsx‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ describe.each(['light', 'dark'] as const)('%s SEO workbench', (theme) => {
4646
const host = mountSeo(theme)
4747
const semantic = resolveSemanticTheme(theme)
4848
const primaryColor = resolvedCssColor(semantic.color.text.primary)
49+
const secondaryColor = resolvedCssColor(semantic.color.text.secondary)
4950
const linkColor = resolvedCssColor(semantic.color.text.link)
5051
const bar = host.querySelector<HTMLElement>(
5152
'nav[aria-label="SEO sections"]',
@@ -56,18 +57,33 @@ describe.each(['light', 'dark'] as const)('%s SEO workbench', (theme) => {
5657

5758
expect(bar).toHaveAttribute('data-workbench-secondary-tabs')
5859
expect(getComputedStyle(bar).height).toBe('44px')
59-
expect(getComputedStyle(bar).borderBottomWidth).not.toBe('1px')
60+
// The shared strip is part of the chrome band, so it closes with the same
61+
// translucent ink hairline the header uses.
62+
expect(getComputedStyle(bar).borderBottomWidth).toBe('1px')
6063
expect(socialTab).toHaveAttribute('data-tsd-selected', 'true')
6164
expect(getComputedStyle(socialTab).backgroundColor).not.toBe(
6265
'rgba(0, 0, 0, 0)',
6366
)
6467

68+
// The network name is a muted uppercase label; the shared title carries the
69+
// primary foreground.
70+
const socialHeadings = [
71+
...host.querySelectorAll<HTMLElement>(
72+
'[data-testid="social-preview-heading"]',
73+
),
74+
]
6575
const socialTitles = [
6676
...host.querySelectorAll<HTMLElement>(
67-
'[data-testid="social-preview-heading"], [data-testid="social-preview-title"]',
77+
'[data-testid="social-preview-title"]',
6878
),
6979
]
80+
expect(socialHeadings.length).toBeGreaterThan(0)
7081
expect(socialTitles.length).toBeGreaterThan(0)
82+
expect(
83+
socialHeadings.every(
84+
(heading) => getComputedStyle(heading).color === secondaryColor,
85+
),
86+
).toBe(true)
7187
expect(
7288
socialTitles.every(
7389
(title) => getComputedStyle(title).color === primaryColor,
@@ -78,18 +94,31 @@ describe.each(['light', 'dark'] as const)('%s SEO workbench', (theme) => {
7894
expect(serpTab).toHaveAttribute('data-tsd-selected', 'true')
7995
expect(serpTab).toHaveAttribute('aria-current', 'page')
8096
expect(socialTab).not.toHaveAttribute('data-tsd-selected')
97+
// Same split as the social cards: the block label is the muted uppercase
98+
// one, the site name carries the primary foreground.
99+
const serpLabels = [
100+
...host.querySelectorAll<HTMLElement>(
101+
'[data-testid="serp-preview-label"]',
102+
),
103+
]
81104
const primaryTitles = [
82105
...host.querySelectorAll<HTMLElement>(
83-
'[data-testid="serp-preview-label"], [data-testid="serp-preview-site-name"]',
106+
'[data-testid="serp-preview-site-name"]',
84107
),
85108
]
86109
const linkTitles = [
87110
...host.querySelectorAll<HTMLElement>(
88111
'[data-testid="serp-preview-title"]',
89112
),
90113
]
114+
expect(serpLabels.length).toBeGreaterThan(0)
91115
expect(primaryTitles.length).toBeGreaterThan(0)
92116
expect(linkTitles).toHaveLength(2)
117+
expect(
118+
serpLabels.every(
119+
(label) => getComputedStyle(label).color === secondaryColor,
120+
),
121+
).toBe(true)
93122
expect(
94123
primaryTitles.every(
95124
(title) => getComputedStyle(title).color === primaryColor,

‎packages/devtools/tests/test-setup.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,29 @@
11
import '@testing-library/jest-dom/vitest'
2+
import { beforeEach } from 'vitest'
3+
4+
// goober finds its single `<style id="_goober">` tag through `window._goober` —
5+
// the global a browser creates for any element with an `id` (named access on the
6+
// window object). jsdom only does that for a few element types, never for
7+
// `<style>`, so goober never finds its sheet and appends a brand new one on
8+
// EVERY `css()` call. One workbench mount makes ~2500 `css()` calls, so the
9+
// document collects ~2500 duplicate stylesheets per mount, every
10+
// `getComputedStyle` walks all of them, and a suite of mounts runs out of heap.
11+
// Giving goober the global a browser would have keeps it to one sheet.
12+
declare global {
13+
interface Window {
14+
_goober?: HTMLStyleElement
15+
}
16+
}
17+
18+
beforeEach(() => {
19+
// Module-level `css()`/`keyframes()` calls run at import time, before the
20+
// first hook, so clear whatever they left behind as well as the previous
21+
// test's sheet.
22+
for (const stale of document.querySelectorAll('style#_goober')) stale.remove()
23+
const sheet = Object.assign(document.createElement('style'), {
24+
innerHTML: ' ',
25+
id: '_goober',
26+
})
27+
document.head.appendChild(sheet)
28+
window._goober = sheet
29+
})

‎packages/devtools/tests/workbench.test.tsx‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import { ClientEventBus } from '@tanstack/devtools-event-bus/client'
1515
import {
1616
PANEL_CLOSE_THRESHOLD,
1717
PANEL_MAX_VIEWPORT_RATIO,
18+
PLUGINS_STRIP_HEIGHT,
19+
WORKBENCH_GUTTER,
1820
WORKBENCH_HEADER_HEIGHT,
1921
} from '../src/utils/constants'
2022
import type {
@@ -352,9 +354,13 @@ describe('workbench', { timeout: 30_000 }, () => {
352354

353355
toggle().click()
354356

355-
// The subheader is the only thing that goes. The panel stays open, keeps
356-
// its height, and the plugin stays mounted and running.
357-
expect(document.querySelector('[data-testid="plugins-strip"]')).toBeNull()
357+
// The subheader is the only thing that goes. It stays mounted so it can
358+
// slide, but folded shut it is inert and out of the accessibility tree.
359+
// The panel stays open, keeps its height, and the plugin stays mounted
360+
// and running.
361+
const strip = document.querySelector('[data-testid="plugins-strip"]')!
362+
expect(strip).toHaveAttribute('data-collapsed', 'true')
363+
expect(strip).toHaveAttribute('aria-hidden', 'true')
358364
expect(outerPanel).toHaveAttribute('data-open', 'true')
359365
expect(outerPanel).toHaveAttribute('data-subheader-collapsed', 'true')
360366
expect(outerPanel.style.transform).toBe('translateY(0px)')
@@ -372,9 +378,8 @@ describe('workbench', { timeout: 30_000 }, () => {
372378

373379
toggle().click()
374380
expect(outerPanel).toHaveAttribute('data-subheader-collapsed', 'false')
375-
expect(
376-
document.querySelector('[data-testid="plugins-strip"]'),
377-
).not.toBeNull()
381+
expect(strip).not.toHaveAttribute('data-collapsed')
382+
expect(strip).not.toHaveAttribute('aria-hidden')
378383
expect(outerPanel.style.height).toBe('417px')
379384
expect(document.querySelector('[data-plugin-mount]')).not.toBeNull()
380385
expect(localStorage.getItem(TANSTACK_DEVTOOLS_STATE)).toBe(storedBefore)
@@ -759,26 +764,30 @@ describe('workbench', { timeout: 30_000 }, () => {
759764
'[data-testid="workbench-destinations"]',
760765
)!
761766

762-
expect(getComputedStyle(header).paddingRight).toBe('8px')
767+
// No trailing gutter: the action icons run all the way to the panel edge.
768+
expect(getComputedStyle(header).paddingRight).toBe('0px')
763769
expect(getComputedStyle(destinations).display).toBe('inline-flex')
764770
expect(Number.parseFloat(getComputedStyle(destinations).gap)).toBe(0)
765771
expect(plugins.parentElement).toBe(destinations)
766772
expect(marketplace.parentElement).toBe(destinations)
767773
expect(seo.parentElement).toBe(destinations)
768774
for (const destination of [plugins, marketplace, seo]) {
769-
expect(getComputedStyle(destination).paddingInline).toBe('12px')
775+
expect(getComputedStyle(destination).paddingInline).toBe('10px')
770776
}
771777
for (const action of actions) {
772778
expect(action).not.toBeNull()
773779
expect(getComputedStyle(action!).width).toBe('36px')
774780
expect(getComputedStyle(action!).height).toBe('36px')
775781
}
776-
expect(getComputedStyle(plugins).backgroundColor).toBe(
777-
getComputedStyle(strip).backgroundColor,
778-
)
782+
// The active destination is a pressed chip sitting on the brand band — a
783+
// translucent overlay, so it reads as raised rather than painting the
784+
// strip's own colour.
779785
expect(getComputedStyle(plugins).backgroundColor).not.toBe(
780786
'rgba(0, 0, 0, 0)',
781787
)
788+
expect(getComputedStyle(plugins).backgroundColor).not.toBe(
789+
getComputedStyle(strip).backgroundColor,
790+
)
782791
expect(getComputedStyle(seo).backgroundColor).toBe('rgba(0, 0, 0, 0)')
783792
expect(getComputedStyle(logo).filter).not.toBe('none')
784793
const stripClass = [...strip.classList][0]!
@@ -794,10 +803,19 @@ describe('workbench', { timeout: 30_000 }, () => {
794803
authoredStyleRules().some(
795804
(rule) =>
796805
rule.selector.includes(`.${stripClass}`) &&
797-
/height:\s*44px/i.test(rule.cssText) &&
806+
new RegExp(`height:\\s*${PLUGINS_STRIP_HEIGHT}px`, 'i').test(
807+
rule.cssText,
808+
) &&
798809
/padding-block:\s*6px/i.test(rule.cssText) &&
799-
/padding-inline-end:\s*8px/i.test(rule.cssText) &&
800-
/scroll-padding-inline-end:\s*8px/i.test(rule.cssText),
810+
// The strip runs on the one workbench gutter, like the header and
811+
// every destination's content.
812+
new RegExp(`padding-inline-end:\\s*${WORKBENCH_GUTTER}px`, 'i').test(
813+
rule.cssText,
814+
) &&
815+
new RegExp(
816+
`scroll-padding-inline-end:\\s*${WORKBENCH_GUTTER}px`,
817+
'i',
818+
).test(rule.cssText),
801819
),
802820
).toBe(true)
803821
expect(
@@ -824,7 +842,9 @@ describe('workbench', { timeout: 30_000 }, () => {
824842
const triggerClasses = [...trigger.classList]
825843
const rules = authoredStyleRules()
826844

827-
expect(getComputedStyle(resize).height).toBe('24px')
845+
expect(getComputedStyle(resize).height).toBe('5px')
846+
// The line has no focus outline of its own — hover and focus paint the bar
847+
// itself, which is what reveals it.
828848
expect(
829849
rules.some(
830850
(rule) =>
@@ -835,17 +855,15 @@ describe('workbench', { timeout: 30_000 }, () => {
835855
expect(
836856
rules.some(
837857
(rule) =>
838-
rule.selector.includes(`.${resizeClass}::after`) &&
839-
!rule.selector.includes(':hover') &&
840-
!rule.selector.includes(':focus-visible') &&
858+
rule.selector === `.${resizeClass}` &&
841859
/background-color:\s*transparent/i.test(rule.cssText),
842860
),
843861
).toBe(true)
844862
for (const state of [':hover', ':focus-visible']) {
845863
expect(
846864
rules.some(
847865
(rule) =>
848-
rule.selector.includes(`.${resizeClass}${state}::after`) &&
866+
rule.selector.includes(`.${resizeClass}${state}`) &&
849867
/background-color:/i.test(rule.cssText) &&
850868
!/transparent/i.test(rule.cssText),
851869
),
@@ -892,8 +910,10 @@ describe('workbench', { timeout: 30_000 }, () => {
892910
expect(baseRule.cssText).toMatch(
893911
/grid-template-rows:\s*36px\s+minmax\(0,\s*1fr\)/i,
894912
)
913+
// The strip row is auto-sized so the strip's own animated height drives it —
914+
// a fixed 44px row would snap instead of sliding.
895915
expect(stripRule.cssText).toMatch(
896-
/grid-template-rows:\s*36px\s+44px\s+minmax\(0,\s*1fr\)/i,
916+
/grid-template-rows:\s*36px\s+auto\s+minmax\(0,\s*1fr\)/i,
897917
)
898918
})
899919

@@ -990,8 +1010,10 @@ describe('workbench', { timeout: 30_000 }, () => {
9901010
const workspace = document.querySelector<HTMLElement>(
9911011
'[data-testid="plugin-marketplace"]',
9921012
)!
993-
expect(workspace.style.height).toBe('100%')
994-
expect(workspace.style.minHeight).toBe('0px')
1013+
// Marketplace carries the constraint in its class rather than inline.
1014+
expect(getComputedStyle(workspace).height).toBe('100%')
1015+
expect(getComputedStyle(workspace).minHeight).toBe('0')
1016+
expect(getComputedStyle(workspace).overflow).toBe('hidden')
9951017
expect(getComputedStyle(workspace.firstElementChild!).overflowY).toBe(
9961018
'auto',
9971019
)

0 commit comments

Comments
 (0)