Expose index and siblings on walk context (#20109)
This PR is a small improvement of the current `walk` implementation
where we will expose the `index` and the `siblings` on the current
context.
During a walk, we walk over objects that contain a `nodes: []` field.
The `ctx.parent` that already exists is a reference to the parent node,
but `ctx.siblings` is a reference to the `ctx.parent.nodes`.
The `ctx.index` is the index of the current node we are walking in the
`siblings` array. This way we can prevent the awkward
`ctx.parent?.nodes.indexOf(node)` which is a bit silly because we
already know the nodes we're walking and its index...
The `ctx.parent` can be `null`, but the `ctx.siblings` will never be
`null`, this can be seen in a situation like this:
```ts
let ast: AstNode = [nodeA, nodeB]
walk(ast, (node, ctx) => {
if (node === nodeA) {
ctx.parent === null; // Because there is no parent
ctx.siblings === ast; // Because that's the current list we're looping over
// Before this PR, we would have to do something like:
let siblings = ctx.parent?.nodes ?? ast
}
})
```
In the above example, the `ast` is a separately variable, but if this
was inlined, we would run into some issues:
```ts
walk([nodeA, nodeB], (node, ctx) => {
if (node === nodeA) {
ctx.parent === null; // Because there is no parent
ctx.siblings === ast; // Because that's the current list we're looping over
// At this point, there is no way to get to the `[nodeA, nodeB]` list
// without moving it to a variable first.
}
})
```
So, this PR doesn't change much, the additional information we track is
already known information that is now exposed to the caller of the
`walk` function.
In this PR we did update some usages and got rid of some awkward
`ctx.parent?.nodes ?? []` and `ctx.parent.nodes.indexOf(…)` usages.
## Test plan
- All tests still pass as expected
This commit is contained in:
parent
982e920df8
commit
749c45ef26
6 changed files with 48 additions and 38 deletions
|
|
@ -211,10 +211,10 @@ function substituteFunctionsInValue(
|
|||
fallbackValues.length > 0 ? handle(path, ValueParser.toCss(fallbackValues)) : handle(path)
|
||||
if (replacement === null) return
|
||||
|
||||
if (ctx.parent) {
|
||||
let idx = ctx.parent.nodes.indexOf(node) - 1
|
||||
{
|
||||
let idx = ctx.index - 1
|
||||
while (idx !== -1) {
|
||||
let previous = ctx.parent.nodes[idx]
|
||||
let previous = ctx.siblings[idx]
|
||||
// Skip the space separator
|
||||
if (previous.kind === 'separator' && previous.value.trim() === '') {
|
||||
idx -= 1
|
||||
|
|
|
|||
|
|
@ -190,6 +190,8 @@ export function cssContext(
|
|||
): VisitContext<AstNode> & { context: Record<string, string | boolean> } {
|
||||
return {
|
||||
depth: ctx.depth,
|
||||
index: ctx.index,
|
||||
siblings: ctx.siblings,
|
||||
get context() {
|
||||
let context: Record<string, string | boolean> = {}
|
||||
for (let child of ctx.path()) {
|
||||
|
|
|
|||
|
|
@ -1048,11 +1048,9 @@ const printArbitraryValueCache = new DefaultMap<string, string>((input) => {
|
|||
'/',
|
||||
])
|
||||
walk(ast, (node, ctx) => {
|
||||
let parentArray = ctx.parent === null ? ast : (ctx.parent.nodes ?? [])
|
||||
|
||||
// Handle operators (e.g.: inside of `calc(…)`)
|
||||
if (node.kind === 'word' && symbols.has(node.value)) {
|
||||
let idx = parentArray.indexOf(node) ?? -1
|
||||
let idx = ctx.index
|
||||
|
||||
// This should not be possible
|
||||
if (idx === -1) return
|
||||
|
|
@ -1060,25 +1058,25 @@ const printArbitraryValueCache = new DefaultMap<string, string>((input) => {
|
|||
// a + b
|
||||
// ^ node
|
||||
// ^ previous (whitespace)
|
||||
let previous = parentArray[idx - 1]
|
||||
let previous = ctx.siblings[idx - 1]
|
||||
if (previous?.kind !== 'separator' || previous.value !== ' ') return
|
||||
|
||||
// a + b
|
||||
// ^ node
|
||||
// ^ next (whitespace)
|
||||
let next = parentArray[idx + 1]
|
||||
let next = ctx.siblings[idx + 1]
|
||||
if (next?.kind !== 'separator' || next.value !== ' ') return
|
||||
|
||||
// a + b
|
||||
// ^ node
|
||||
// ^ previous (node)
|
||||
let previousPrevious = parentArray[idx - 2]
|
||||
let previousPrevious = ctx.siblings[idx - 2]
|
||||
if (previousPrevious && symbols.has(previousPrevious.value)) return
|
||||
|
||||
// a + b
|
||||
// ^ node
|
||||
// ^ next (node)
|
||||
let nextNext = parentArray[idx + 2]
|
||||
let nextNext = ctx.siblings[idx + 2]
|
||||
if (nextNext && symbols.has(nextNext.value)) return
|
||||
|
||||
drop.add(previous)
|
||||
|
|
@ -1087,7 +1085,7 @@ const printArbitraryValueCache = new DefaultMap<string, string>((input) => {
|
|||
|
||||
// Leading and trailing whitespace
|
||||
else if (node.kind === 'separator' && node.value.length > 0 && node.value.trim() === '') {
|
||||
if (parentArray[0] === node || parentArray[parentArray.length - 1] === node) {
|
||||
if (ctx.siblings[0] === node || ctx.siblings[ctx.siblings.length - 1] === node) {
|
||||
drop.add(node)
|
||||
}
|
||||
}
|
||||
|
|
@ -1101,7 +1099,7 @@ const printArbitraryValueCache = new DefaultMap<string, string>((input) => {
|
|||
// Wrap custom functions starting with `--`, in parentheses if preceeded by
|
||||
// a symbol. E.g.: `calc(100%---spacing(2))` → `calc(100%-(--spacing(2)))`
|
||||
else if (node.kind === 'function' && node.value.startsWith('--')) {
|
||||
let idx = parentArray.indexOf(node) ?? -1
|
||||
let idx = ctx.index
|
||||
|
||||
// When it's the first argument, then we don't have to wrap it in `(…)`
|
||||
//
|
||||
|
|
@ -1115,7 +1113,7 @@ const printArbitraryValueCache = new DefaultMap<string, string>((input) => {
|
|||
//
|
||||
// E.g.: `min(100%,--spacing(2))` is readable, in fact
|
||||
// `min(100%,(--spacing(2)))` would make it worse
|
||||
let previous = parentArray[idx - 1]
|
||||
let previous = ctx.siblings[idx - 1]
|
||||
if (previous?.kind === 'separator' && previous.value === ',') return
|
||||
|
||||
// When it's part of a bigger list, aka no special symbols were used, then
|
||||
|
|
@ -1123,7 +1121,7 @@ const printArbitraryValueCache = new DefaultMap<string, string>((input) => {
|
|||
//
|
||||
// E.g.: `shadow-[inset_0px_1px_--theme(--color-white/15%)]`, wrapping would look unnecessary:
|
||||
// `shadow-[inset_0px_1px_(--theme(--color-white/15%))]`
|
||||
let previousPrevious = parentArray[idx - 2]
|
||||
let previousPrevious = ctx.siblings[idx - 2]
|
||||
if (previousPrevious && !symbols.has(previousPrevious.value)) return
|
||||
|
||||
return WalkAction.ReplaceSkip({
|
||||
|
|
|
|||
|
|
@ -980,10 +980,10 @@ function substituteFunctionsInValue(
|
|||
fallbackValues.length > 0 ? handle(path, ValueParser.toCss(fallbackValues)) : handle(path)
|
||||
if (replacement === null) return
|
||||
|
||||
if (ctx.parent) {
|
||||
let idx = ctx.parent.nodes.indexOf(node) - 1
|
||||
{
|
||||
let idx = ctx.index - 1
|
||||
while (idx !== -1) {
|
||||
let previous = ctx.parent.nodes[idx]
|
||||
let previous = ctx.siblings[idx]
|
||||
// Skip the space separator
|
||||
if (previous.kind === 'separator' && previous.value.trim() === '') {
|
||||
idx -= 1
|
||||
|
|
@ -2405,7 +2405,7 @@ function canonicalizeAst(designSystem: DesignSystem, ast: AstNode[], options: Si
|
|||
// Ignore `--tw-{property}` if `{property}` exists with the same value
|
||||
if (node.property.startsWith('--tw-')) {
|
||||
if (
|
||||
(ctx.parent?.nodes ?? []).some(
|
||||
ctx.siblings.some(
|
||||
(sibling) =>
|
||||
sibling.kind === 'declaration' &&
|
||||
node.value === sibling.value &&
|
||||
|
|
|
|||
|
|
@ -956,33 +956,35 @@ describe('AST Enter & Exit', () => {
|
|||
let visited: string[] = []
|
||||
walk(ast, {
|
||||
enter(node, ctx) {
|
||||
visited.push(`${' '.repeat(ctx.depth)} Enter(${node.kind})`)
|
||||
expect(ctx.index).toEqual(ctx.siblings.indexOf(node))
|
||||
visited.push(`${' '.repeat(ctx.depth)} Enter(${node.kind} @ ${ctx.index})`)
|
||||
},
|
||||
exit(node, ctx) {
|
||||
visited.push(`${' '.repeat(ctx.depth)} Exit(${node.kind})`)
|
||||
expect(ctx.index).toEqual(ctx.siblings.indexOf(node))
|
||||
visited.push(`${' '.repeat(ctx.depth)} Exit(${node.kind} @ ${ctx.index})`)
|
||||
},
|
||||
})
|
||||
|
||||
expect(`\n${visited.join('\n')}\n`).toMatchInlineSnapshot(`
|
||||
"
|
||||
Enter(a)
|
||||
Enter(b)
|
||||
Enter(c)
|
||||
Exit(c)
|
||||
Exit(b)
|
||||
Enter(d)
|
||||
Enter(e)
|
||||
Enter(f)
|
||||
Exit(f)
|
||||
Exit(e)
|
||||
Exit(d)
|
||||
Enter(g)
|
||||
Enter(h)
|
||||
Exit(h)
|
||||
Exit(g)
|
||||
Exit(a)
|
||||
Enter(i)
|
||||
Exit(i)
|
||||
Enter(a @ 0)
|
||||
Enter(b @ 0)
|
||||
Enter(c @ 0)
|
||||
Exit(c @ 0)
|
||||
Exit(b @ 0)
|
||||
Enter(d @ 1)
|
||||
Enter(e @ 0)
|
||||
Enter(f @ 0)
|
||||
Exit(f @ 0)
|
||||
Exit(e @ 0)
|
||||
Exit(d @ 1)
|
||||
Enter(g @ 2)
|
||||
Enter(h @ 0)
|
||||
Exit(h @ 0)
|
||||
Exit(g @ 2)
|
||||
Exit(a @ 0)
|
||||
Enter(i @ 1)
|
||||
Exit(i @ 1)
|
||||
"
|
||||
`)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -36,6 +36,8 @@ type Parent<T> = T & { nodes: T[] }
|
|||
export interface VisitContext<T> {
|
||||
parent: Parent<T> | null
|
||||
depth: number
|
||||
index: number
|
||||
siblings: T[]
|
||||
path: () => T[]
|
||||
}
|
||||
|
||||
|
|
@ -68,6 +70,8 @@ function walkImplementation<T extends { nodes?: T[] }>(
|
|||
let ctx: VisitContext<T> = {
|
||||
parent: null,
|
||||
depth: 0,
|
||||
index: 0,
|
||||
siblings: ast,
|
||||
path() {
|
||||
let path: T[] = []
|
||||
|
||||
|
|
@ -99,9 +103,12 @@ function walkImplementation<T extends { nodes?: T[] }>(
|
|||
}
|
||||
|
||||
ctx.parent = parent
|
||||
ctx.siblings = nodes
|
||||
|
||||
// Enter phase (offsets are positive)
|
||||
if (offset >= 0) {
|
||||
ctx.index = offset
|
||||
|
||||
let node = nodes[offset]
|
||||
let result = enter(node, ctx) ?? WalkAction.Continue
|
||||
|
||||
|
|
@ -155,6 +162,7 @@ function walkImplementation<T extends { nodes?: T[] }>(
|
|||
|
||||
// Exit phase for nodes[~offset]
|
||||
let index = ~offset // Two's complement to get original offset
|
||||
ctx.index = index
|
||||
let node = nodes[index]
|
||||
|
||||
let result = exit(node, ctx) ?? WalkAction.Continue
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue