Commit graph

4 commits

Author SHA1 Message Date
Robin Malfait
acb27ef9e9
Generalize the walk implementation (#19126)
This PR generalizes the `walk` implementations we have. What's important
here is that we currently have multiple `walk` implementations, one for
the AST, one for the `SelectorParser`, one for the `ValueParser`.

Sometimes, we also need to go up the tree in a depth-first manner. For
that, we have `walkDepth` implementations.

The funny thing is, all these implementations are very very similar,
even the kinds of trees are very similar. They are just objects with
`nodes: []` as children.

So this PR introduces a generic `walk` function that can work on all of
these trees.

There are also some situations where you need to go down and back up the
tree. For this reason, we added an `enter` and `exit` phase:

```ts
walk(ast, {
  enter(node, ctx) {},
  exit(node, ctx) {},
})
```

This means that you don't need to `walk(ast)` and later `walkDepth(ast)`
in case you wanted to do something _after_ visiting all nodes.

The API of these walk functions also slightly changed to fix some
problems we've had before. One is the `replaceWith` function. You could
technically call it multiple times, but that doesn't make sense so
instead you always have to return an explicit `WalkAction`. The
possibilities are:

```ts
// The ones we already had
WalkAction.Continue // Continue walking as normal, the default behavior
WalkAction.Skip // Skip walking the `nodes` of the current node
WalkAction.Stop // Stop the entire walk

// The new ones
WalkAction.Replace(newNode) // Replace the current node, and continue walking the new node(s)
WalkAction.ReplaceSkip(newNode) // Replace the current node, but don't walk the new node(s)
WalkAction.ReplaceStop(newNode) // Replace the current node, but stop the entire walk
```

To make sure that we can walk in both directions, and to make sure we
have proper control over when to walk which nodes, the `walk` function
is implemented in an iterative manner using a stack instead of
recursion.

This also means that a `WalkAction.Stop` or `WalkAction.ReplaceStop`
will immediately stop the walk, without unwinding the entire call stack.

Some notes: 
- The CSS AST does have `context` nodes, for this we can build up the
context lazily when we need it. I added a `cssContext(ctx)` that gives
you an enhanced context including the `context` object that you can read
information from.
- The second argument of the `walk` function can still be a normal
function, which is equivalent to `{ enter: fn }`.

Let's also take a look at some numbers. With this new implementation,
each `walk` is roughly ~1.3-1.5x faster than before. If you look at the
memory usage (especially in Bun) we go from `~2.2GB` peak memory usage,
to `~300mb` peak memory usage.

Some benchmarks on small and big trees (M1 Max):

<img width="2062" height="1438" alt="image"
src="https://github.com/user-attachments/assets/5ec8c22a-9de8-4e08-869a-18c0d30eb7e8"
/>

<img width="2062" height="1246" alt="image"
src="https://github.com/user-attachments/assets/e89d4b8e-29ca-4aee-8fd2-b7c043d3bbf4"
/>

We also ran some benchmarks on @thecrypticace's M3 Max:

<img width="1598" height="1452" alt="image"
src="https://github.com/user-attachments/assets/3b06b6fe-2497-4f24-a428-1a0e2af3896a"
/>

In node the memory difference isn't that big, but the performance itself
is still better:

<img width="2034" height="1586" alt="image"
src="https://github.com/user-attachments/assets/ef28ae14-b53e-4912-9621-531f3b02898f"
/>


In summary:

1. Single `walk` implementation for multiple use cases
2. Support for `enter` and `exit` phases
3. New `WalkAction` possibilities for better control
4. Overall better performance
5. ... and lower memory usage

## Test plan

1. All tests still pass (but had to adjust some of the APIs if `walk`
was used inside tests).
2. Added new tests for the `walk` implementation
3. Ran local benchmarks to verify the performance improvements
2025-10-15 21:28:12 +02:00
Philipp Spiess
deb33a93ab
Vite: Don't rebase urls that appear to be aliases (#16078)
Closes #16039

This PR changes our URL rebasing logic used with Vite so that it does
not rebase URLs that look like common alias paths (e.g. urls starting in
`~`, `@` or `#`, etc.). Unfortunately this is only an approximation and
you can configure an alias for a path that starts with a regular
alphabetical character (e.g. `foo` => `./my/foo`) so this isn't a
perfect fix, however in practice most aliases will be prefixed with a
symbol to make it clear that it's an alias anyways.

One alternative we have considered is to only rebase URLs that we know
are relative (so they need to start with a `.`). This, however, will
break common CSS use cases where urls are loaded like this:

```css
background: image-set(
  url('image1.jpg') 1x,
  url('image2.jpg') 2x
);
``` 

So making this change felt like we only trade one GitHub issue for
another one.

In a more ideal scenario we try to resolve the URL with the Vite
resolver (we have to run the resolver and can't rely on the `resolve`
setting alone due to packages like
[`vite-tsconfig-paths`](https://www.npmjs.com/package/vite-tsconfig-paths)),
however even then we can have relative paths being resolvable to
different files based on wether they were rebased or not (e.g. when an
image with the same filename exists in two different paths).

So ultimately we settled on extending the already existing blocklist
(which we have taken from the Vite implementation) for now.

## Test plan

- Added unit test and it was tested with the Vite playground.

---------

Co-authored-by: Robin Malfait <malfait.robin@gmail.com>
2025-01-31 15:23:32 +01:00
Philipp Spiess
667af255b3
Vite: Don't rebase absolute url()s (#15275)
Closes #15269

This PR fixes an issue where our Vite extension was rebasing absolute
urls inside `@import`-ed files. We forgot to cover this when we
implemented the URL rebasing.

## Test Plan

We validated that this fixes the repro in #15269:

<img width="851" alt="Screenshot 2024-12-02 at 18 07 35"
src="https://github.com/user-attachments/assets/3b2c2be3-1f73-469e-9f64-301c6b948b02">

Also added a unit test for this.

Co-authored-by: Jordan Pittman <jordan@cryptica.me>
2024-12-02 14:23:38 -05:00
Jordan Pittman
e82b316c61
Rewrite urls in CSS files when using Vite (#14877)
Fixes #14784

This is an alternative to #14850 in which we actually perform url
rewriting / rebasing ourselves. We ported a large portion of the
URL-rewriting code from Vite (with attribution) to use here with some
minor modifications. We've added test cases for the url rewriting so
verifying individual cases is easy. We also wrote integration tests for
Vite that use PostCSS and Lightning CSS that verify that files are found
and inlined or relocated/renamed as necessary.

We also did some manual testing in the Playground to verify that this
works as expected across several CSS files and directories which you can
see a screenshot from here:

<img width="1344" alt="Screenshot 2024-11-05 at 10 25 16"
src="https://github.com/user-attachments/assets/ff0b3ac8-cdc9-4e26-af79-36396a5b77b9">

---------

Co-authored-by: Philipp Spiess <hello@philippspiess.com>
2024-11-07 09:51:58 -05:00