Skip to content

Commit 39a5180

Browse files
philipp-spiessphilipp-spiess
committed
Properly skip over nodes when using replaceNode 
1 parent d566dbd commit 39a5180

File tree

4 files changed

+371
-16
lines changed

4 files changed

+371
-16
lines changed

‎packages/tailwindcss/src/ast.test.ts

Lines changed: 318 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
import { expect, it } from 'vitest'
2-
import { context, decl, optimizeAst, styleRule, toCss, walk, WalkAction } from './ast'
2+
import {
3+
atRule,
4+
context,
5+
decl,
6+
optimizeAst,
7+
styleRule,
8+
toCss,
9+
walk,
10+
WalkAction,
11+
type AstNode,
12+
} from './ast'
313
import * as CSS from './css-parser'
414

515
const css = String.raw
@@ -188,3 +198,310 @@ it('should not emit empty rules once optimized', () => {
188198
"
189199
`)
190200
})
201+
202+
it('should only visit children once when calling `replaceWith` with single element array', () => {
203+
let visited = new Set()
204+
205+
let ast = [
206+
atRule('@media', '', [styleRule('.foo', [decl('color', 'blue')])]),
207+
styleRule('.bar', [decl('color', 'blue')]),
208+
]
209+
210+
walk(ast, (node, { replaceWith }) => {
211+
if (visited.has(node)) {
212+
throw new Error('Visited node twice')
213+
}
214+
visited.add(node)
215+
216+
if (node.kind === 'at-rule') replaceWith(node.nodes)
217+
})
218+
})
219+
220+
it('should only visit children once when calling `replaceWith` with multi-element array', () => {
221+
let visited = new Set()
222+
223+
let ast = [
224+
atRule('@media', '', [
225+
context({}, [
226+
styleRule('.foo', [decl('color', 'red')]),
227+
styleRule('.baz', [decl('color', 'blue')]),
228+
]),
229+
]),
230+
styleRule('.bar', [decl('color', 'green')]),
231+
]
232+
233+
walk(ast, (node, { replaceWith }) => {
234+
let key = id(node)
235+
if (visited.has(key)) {
236+
throw new Error('Visited node twice')
237+
}
238+
visited.add(key)
239+
240+
if (node.kind === 'at-rule') replaceWith(node.nodes)
241+
})
242+
243+
expect(visited).toMatchInlineSnapshot(`
244+
Set {
245+
"@media ",
246+
".foo",
247+
"color: red",
248+
".baz",
249+
"color: blue",
250+
".bar",
251+
"color: green",
252+
}
253+
`)
254+
})
255+
256+
it('should never visit children when calling `replaceWith` with `WalkAction.Skip`', () => {
257+
let visited = new Set()
258+
259+
let inner = styleRule('.foo', [decl('color', 'blue')])
260+
261+
let ast = [atRule('@media', '', [inner]), styleRule('.bar', [decl('color', 'blue')])]
262+
263+
walk(ast, (node, { replaceWith }) => {
264+
visited.add(node)
265+
266+
if (node.kind === 'at-rule') {
267+
replaceWith(node.nodes)
268+
return WalkAction.Skip
269+
}
270+
})
271+
272+
expect(visited).not.toContain(inner)
273+
expect(visited).toMatchInlineSnapshot(`
274+
Set {
275+
{
276+
"kind": "at-rule",
277+
"name": "@media",
278+
"nodes": [
279+
{
280+
"kind": "rule",
281+
"nodes": [
282+
{
283+
"important": false,
284+
"kind": "declaration",
285+
"property": "color",
286+
"value": "blue",
287+
},
288+
],
289+
"selector": ".foo",
290+
},
291+
],
292+
"params": "",
293+
},
294+
{
295+
"kind": "rule",
296+
"nodes": [
297+
{
298+
"important": false,
299+
"kind": "declaration",
300+
"property": "color",
301+
"value": "blue",
302+
},
303+
],
304+
"selector": ".bar",
305+
},
306+
{
307+
"important": false,
308+
"kind": "declaration",
309+
"property": "color",
310+
"value": "blue",
311+
},
312+
}
313+
`)
314+
})
315+
316+
it('should skip the correct number of children based on the the replaced children nodes', () => {
317+
{
318+
let ast = [
319+
decl('--index', '0'),
320+
decl('--index', '1'),
321+
decl('--index', '2'),
322+
decl('--index', '3'),
323+
decl('--index', '4'),
324+
]
325+
let visited: string[] = []
326+
walk(ast, (node, { replaceWith }) => {
327+
visited.push(id(node))
328+
if (node.kind === 'declaration' && node.value === '2') {
329+
replaceWith([])
330+
return WalkAction.Skip
331+
}
332+
})
333+
334+
expect(visited).toMatchInlineSnapshot(`
335+
[
336+
"--index: 0",
337+
"--index: 1",
338+
"--index: 2",
339+
"--index: 3",
340+
"--index: 4",
341+
]
342+
`)
343+
}
344+
345+
{
346+
let ast = [
347+
decl('--index', '0'),
348+
decl('--index', '1'),
349+
decl('--index', '2'),
350+
decl('--index', '3'),
351+
decl('--index', '4'),
352+
]
353+
let visited: string[] = []
354+
walk(ast, (node, { replaceWith }) => {
355+
visited.push(id(node))
356+
if (node.kind === 'declaration' && node.value === '2') {
357+
replaceWith([])
358+
return WalkAction.Continue
359+
}
360+
})
361+
362+
expect(visited).toMatchInlineSnapshot(`
363+
[
364+
"--index: 0",
365+
"--index: 1",
366+
"--index: 2",
367+
"--index: 3",
368+
"--index: 4",
369+
]
370+
`)
371+
}
372+
373+
{
374+
let ast = [
375+
decl('--index', '0'),
376+
decl('--index', '1'),
377+
decl('--index', '2'),
378+
decl('--index', '3'),
379+
decl('--index', '4'),
380+
]
381+
let visited: string[] = []
382+
walk(ast, (node, { replaceWith }) => {
383+
visited.push(id(node))
384+
if (node.kind === 'declaration' && node.value === '2') {
385+
replaceWith([decl('--index', '2.1')])
386+
return WalkAction.Skip
387+
}
388+
})
389+
390+
expect(visited).toMatchInlineSnapshot(`
391+
[
392+
"--index: 0",
393+
"--index: 1",
394+
"--index: 2",
395+
"--index: 3",
396+
"--index: 4",
397+
]
398+
`)
399+
}
400+
401+
{
402+
let ast = [
403+
decl('--index', '0'),
404+
decl('--index', '1'),
405+
decl('--index', '2'),
406+
decl('--index', '3'),
407+
decl('--index', '4'),
408+
]
409+
let visited: string[] = []
410+
walk(ast, (node, { replaceWith }) => {
411+
visited.push(id(node))
412+
if (node.kind === 'declaration' && node.value === '2') {
413+
replaceWith([decl('--index', '2.1')])
414+
return WalkAction.Continue
415+
}
416+
})
417+
418+
expect(visited).toMatchInlineSnapshot(`
419+
[
420+
"--index: 0",
421+
"--index: 1",
422+
"--index: 2",
423+
"--index: 2.1",
424+
"--index: 3",
425+
"--index: 4",
426+
]
427+
`)
428+
}
429+
430+
{
431+
let ast = [
432+
decl('--index', '0'),
433+
decl('--index', '1'),
434+
decl('--index', '2'),
435+
decl('--index', '3'),
436+
decl('--index', '4'),
437+
]
438+
let visited: string[] = []
439+
walk(ast, (node, { replaceWith }) => {
440+
visited.push(id(node))
441+
if (node.kind === 'declaration' && node.value === '2') {
442+
replaceWith([decl('--index', '2.1'), decl('--index', '2.2')])
443+
return WalkAction.Skip
444+
}
445+
})
446+
447+
expect(visited).toMatchInlineSnapshot(`
448+
[
449+
"--index: 0",
450+
"--index: 1",
451+
"--index: 2",
452+
"--index: 3",
453+
"--index: 4",
454+
]
455+
`)
456+
}
457+
458+
{
459+
let ast = [
460+
decl('--index', '0'),
461+
decl('--index', '1'),
462+
decl('--index', '2'),
463+
decl('--index', '3'),
464+
decl('--index', '4'),
465+
]
466+
let visited: string[] = []
467+
walk(ast, (node, { replaceWith }) => {
468+
visited.push(id(node))
469+
if (node.kind === 'declaration' && node.value === '2') {
470+
replaceWith([decl('--index', '2.1'), decl('--index', '2.2')])
471+
return WalkAction.Continue
472+
}
473+
})
474+
475+
expect(visited).toMatchInlineSnapshot(`
476+
[
477+
"--index: 0",
478+
"--index: 1",
479+
"--index: 2",
480+
"--index: 2.1",
481+
"--index: 2.2",
482+
"--index: 3",
483+
"--index: 4",
484+
]
485+
`)
486+
}
487+
})
488+
489+
function id(node: AstNode) {
490+
switch (node.kind) {
491+
case 'at-rule':
492+
return `${node.name} ${node.params}`
493+
case 'rule':
494+
return node.selector
495+
case 'context':
496+
return '<context>'
497+
case 'at-root':
498+
return '<at-root>'
499+
case 'declaration':
500+
return `${node.property}: ${node.value}`
501+
case 'comment':
502+
return `// ${node.value}`
503+
default:
504+
node satisfies never
505+
throw new Error('Unknown node kind')
506+
}
507+
}

‎packages/tailwindcss/src/ast.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,39 +137,54 @@ export function walk(
137137
}
138138

139139
path.push(node)
140+
let replacedNode = false
141+
let replacedNodeOffset = 0
140142
let status =
141143
visit(node, {
142144
parent,
143145
context,
144146
path,
145147
replaceWith(newNode) {
148+
replacedNode = true
149+
146150
if (Array.isArray(newNode)) {
147151
if (newNode.length === 0) {
148152
ast.splice(i, 1)
153+
replacedNodeOffset = 0
149154
} else if (newNode.length === 1) {
150155
ast[i] = newNode[0]
156+
replacedNodeOffset = 1
151157
} else {
152158
ast.splice(i, 1, ...newNode)
159+
replacedNodeOffset = newNode.length
153160
}
154161
} else {
155162
ast[i] = newNode
163+
replacedNodeOffset = 1
156164
}
157-
158-
// We want to visit the newly replaced node(s), which start at the
159-
// current index (i). By decrementing the index here, the next loop
160-
// will process this position (containing the replaced node) again.
161-
i--
162165
},
163166
}) ?? WalkAction.Continue
164167
path.pop()
165168

169+
// We want to visit or skip the newly replaced node(s), which start at the
170+
// current index (i). By decrementing the index here, the next loop will
171+
// process this position (containing the replaced node) again.
172+
if (replacedNode) {
173+
if (status === WalkAction.Continue) {
174+
i--
175+
} else {
176+
i += replacedNodeOffset - 1
177+
}
178+
continue
179+
}
180+
166181
// Stop the walk entirely
167182
if (status === WalkAction.Stop) return WalkAction.Stop
168183

169184
// Skip visiting the children of this node
170185
if (status === WalkAction.Skip) continue
171186

172-
if (node.kind === 'rule' || node.kind === 'at-rule') {
187+
if ('nodes' in node) {
173188
path.push(node)
174189
let result = walk(node.nodes, visit, path, context)
175190
path.pop()

0 commit comments

Comments
 (0)