Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PostCSS plugin to fix relative @content and @plugin paths in @imported files #14063

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented Jul 26, 2024

We noticed an issue that happened when handling relative file imports in the @plugin and the upcoming @content APIs. The problem arises from relative files that are inside @imported stylesheets. Take, for example, the following folder structure:

/* src/index.css */
@import "./dir/index.css";
/* src/dir/index.css */
@plugin "../../plugin.ts";

It's expected that the path is relative to the CSS file that defined it. However, right now, we use postcss-import to flatten the CSS file before running the tailwind build step. This causes these custom-properties to be inlined in a flat file which removes the information of which file is being referred:

/* src/flat.css */
@plugin "../../plugin.ts"; /* <- This is now pointing to the wrong file */

There are generally two approaches that we can do to solve this:

  1. Handle @import flattening inside tailwindcss: While generally this would give us more freedom and less dependencies, this would require some work to get all edge cases right. We need to support layers/conditional imports and also handle all relative urls for properties like background-image.
  2. Rewrite relative paths as a separate postcss visitor: The approach this PR takes is instead to implement a custom postcss plugin that uses the AST to rewrite relative references inside @plugin and @content. This has the benefit of requiring little changes to our existing APIs. The rule is only enabled for relative references inside @plugin and @content, so the surface of this rule is very small.

We can use this plugin inside all three current clients:

  • @tailwindcss/postcss obviously already uses postcss
  • @tailwindcss/cli also uses postcss to handle @import flattening
  • @tailwindcss/vite allows us to add custom postcss rules via the CSS pipeline. There are a few cases that we handle with care (e.g. in vite you can pass a string to the postcss config which is supposed to load the config from a file).

Test plan

To validate the changes, we have added both a list of unit test cases to the plugin itself as well as verified that all three clients are working as expected:

  • @tailwindcss/postcss now has an explicit test for this behavior
  • @tailwindcss/cli and @tailwindcss/vite were manually tested by updating the vite playground. The CLI was run with --cwd playgrounds/vite/ -i ./src/app.css -o foo.css:
    Screenshot 2024-07-29 at 11 35 59

@RobinMalfait RobinMalfait changed the base branch from next to feat/add-content-support July 29, 2024 11:05
@philipp-spiess philipp-spiess marked this pull request as ready for review July 29, 2024 12:16
@RobinMalfait RobinMalfait mentioned this pull request Jul 29, 2024
8 tasks
@philipp-spiess philipp-spiess force-pushed the feat/fix-relative-at-content-and-at-plugin branch from b984a60 to a92ac09 Compare July 29, 2024 15:27
@philipp-spiess philipp-spiess force-pushed the feat/fix-relative-at-content-and-at-plugin branch from a92ac09 to 2c34553 Compare July 29, 2024 15:30
@philipp-spiess philipp-spiess merged commit 1ee39a6 into feat/add-content-support Jul 29, 2024
@philipp-spiess philipp-spiess deleted the feat/fix-relative-at-content-and-at-plugin branch July 29, 2024 15:57
RobinMalfait pushed a commit that referenced this pull request Jul 30, 2024
…`@import`ed files (#14063)

We noticed an issue that happened when handling relative file imports in
the `@plugin` and the upcoming `@content` APIs. The problem arises from
relative files that are inside `@import`ed stylesheets. Take, for
example, the following folder structure:

```css
/* src/index.css */
@import "./dir/index.css";
```
```css
/* src/dir/index.css */
@plugin "../../plugin.ts";
```

It's expected that the path is relative to the CSS file that defined it.
However, right now, we use
[`postcss-import`](https://github.com/postcss/postcss-import) to flatten
the CSS file before running the tailwind build step. This causes these
custom-properties to be inlined in a flat file which removes the
information of which file is being referred:

```css
/* src/flat.css */
@plugin "../../plugin.ts"; /* <- This is now pointing to the wrong file */
```

There are generally two approaches that we can do to solve this:

1. **Handle `@import` flattening inside tailwindcss:** While generally
this would give us more freedom and less dependencies, this would
require some work to get all edge cases right. We need to support
layers/conditional imports and also handle all relative urls for
properties like `background-image`.
2. **Rewrite relative paths as a separate postcss visitor:** The
approach this PR takes is instead to implement a custom postcss plugin
that uses the AST to rewrite relative references inside `@plugin` and
`@content`. This has the benefit of requiring little changes to our
existing APIs. The rule is only enabled for relative references inside
`@plugin` and `@content`, so the surface of this rule is very small.

We can use this plugin inside all three current clients:

- `@tailwindcss/postcss` obviously already uses postcss
- `@tailwindcss/cli` also uses postcss to handle `@import` flattening
- `@tailwindcss/vite` allows us to add custom postcss rules via the CSS
pipeline. There are a few cases that we handle with care (e.g. in vite
you can pass a string to the postcss config which is supposed to load
the config from a file).

To validate the changes, we have added both a list of unit test cases to
the plugin itself as well as verified that all three clients are working
as expected:

- `@tailwindcss/postcss` now has an explicit test for this behavior
- `@tailwindcss/cli` and `@tailwindcss/vite` were manually tested by
updating the vite playground. The CLI was run with `--cwd
playgrounds/vite/ -i ./src/app.css -o foo.css`:
<img width="531" alt="Screenshot 2024-07-29 at 11 35 59"
src="https://github.com/user-attachments/assets/78f0acdc-a46c-4c6c-917a-2916417b1001">
philipp-spiess added a commit that referenced this pull request Jul 30, 2024
…`@import`ed files (#14063)

We noticed an issue that happened when handling relative file imports in
the `@plugin` and the upcoming `@content` APIs. The problem arises from
relative files that are inside `@import`ed stylesheets. Take, for
example, the following folder structure:

```css
/* src/index.css */
@import "./dir/index.css";
```
```css
/* src/dir/index.css */
@plugin "../../plugin.ts";
```

It's expected that the path is relative to the CSS file that defined it.
However, right now, we use
[`postcss-import`](https://github.com/postcss/postcss-import) to flatten
the CSS file before running the tailwind build step. This causes these
custom-properties to be inlined in a flat file which removes the
information of which file is being referred:

```css
/* src/flat.css */
@plugin "../../plugin.ts"; /* <- This is now pointing to the wrong file */
```

There are generally two approaches that we can do to solve this:

1. **Handle `@import` flattening inside tailwindcss:** While generally
this would give us more freedom and less dependencies, this would
require some work to get all edge cases right. We need to support
layers/conditional imports and also handle all relative urls for
properties like `background-image`.
2. **Rewrite relative paths as a separate postcss visitor:** The
approach this PR takes is instead to implement a custom postcss plugin
that uses the AST to rewrite relative references inside `@plugin` and
`@content`. This has the benefit of requiring little changes to our
existing APIs. The rule is only enabled for relative references inside
`@plugin` and `@content`, so the surface of this rule is very small.

We can use this plugin inside all three current clients:

- `@tailwindcss/postcss` obviously already uses postcss
- `@tailwindcss/cli` also uses postcss to handle `@import` flattening
- `@tailwindcss/vite` allows us to add custom postcss rules via the CSS
pipeline. There are a few cases that we handle with care (e.g. in vite
you can pass a string to the postcss config which is supposed to load
the config from a file).

To validate the changes, we have added both a list of unit test cases to
the plugin itself as well as verified that all three clients are working
as expected:

- `@tailwindcss/postcss` now has an explicit test for this behavior
- `@tailwindcss/cli` and `@tailwindcss/vite` were manually tested by
updating the vite playground. The CLI was run with `--cwd
playgrounds/vite/ -i ./src/app.css -o foo.css`:
<img width="531" alt="Screenshot 2024-07-29 at 11 35 59"
src="https://github.com/user-attachments/assets/78f0acdc-a46c-4c6c-917a-2916417b1001">
philipp-spiess added a commit that referenced this pull request Aug 2, 2024
…`@import`ed files (#14063)

We noticed an issue that happened when handling relative file imports in
the `@plugin` and the upcoming `@content` APIs. The problem arises from
relative files that are inside `@import`ed stylesheets. Take, for
example, the following folder structure:

```css
/* src/index.css */
@import "./dir/index.css";
```
```css
/* src/dir/index.css */
@plugin "../../plugin.ts";
```

It's expected that the path is relative to the CSS file that defined it.
However, right now, we use
[`postcss-import`](https://github.com/postcss/postcss-import) to flatten
the CSS file before running the tailwind build step. This causes these
custom-properties to be inlined in a flat file which removes the
information of which file is being referred:

```css
/* src/flat.css */
@plugin "../../plugin.ts"; /* <- This is now pointing to the wrong file */
```

There are generally two approaches that we can do to solve this:

1. **Handle `@import` flattening inside tailwindcss:** While generally
this would give us more freedom and less dependencies, this would
require some work to get all edge cases right. We need to support
layers/conditional imports and also handle all relative urls for
properties like `background-image`.
2. **Rewrite relative paths as a separate postcss visitor:** The
approach this PR takes is instead to implement a custom postcss plugin
that uses the AST to rewrite relative references inside `@plugin` and
`@content`. This has the benefit of requiring little changes to our
existing APIs. The rule is only enabled for relative references inside
`@plugin` and `@content`, so the surface of this rule is very small.

We can use this plugin inside all three current clients:

- `@tailwindcss/postcss` obviously already uses postcss
- `@tailwindcss/cli` also uses postcss to handle `@import` flattening
- `@tailwindcss/vite` allows us to add custom postcss rules via the CSS
pipeline. There are a few cases that we handle with care (e.g. in vite
you can pass a string to the postcss config which is supposed to load
the config from a file).

To validate the changes, we have added both a list of unit test cases to
the plugin itself as well as verified that all three clients are working
as expected:

- `@tailwindcss/postcss` now has an explicit test for this behavior
- `@tailwindcss/cli` and `@tailwindcss/vite` were manually tested by
updating the vite playground. The CLI was run with `--cwd
playgrounds/vite/ -i ./src/app.css -o foo.css`:
<img width="531" alt="Screenshot 2024-07-29 at 11 35 59"
src="https://github.com/user-attachments/assets/78f0acdc-a46c-4c6c-917a-2916417b1001">
philipp-spiess added a commit that referenced this pull request Aug 5, 2024
…`@import`ed files (#14063)

We noticed an issue that happened when handling relative file imports in
the `@plugin` and the upcoming `@content` APIs. The problem arises from
relative files that are inside `@import`ed stylesheets. Take, for
example, the following folder structure:

```css
/* src/index.css */
@import "./dir/index.css";
```
```css
/* src/dir/index.css */
@plugin "../../plugin.ts";
```

It's expected that the path is relative to the CSS file that defined it.
However, right now, we use
[`postcss-import`](https://github.com/postcss/postcss-import) to flatten
the CSS file before running the tailwind build step. This causes these
custom-properties to be inlined in a flat file which removes the
information of which file is being referred:

```css
/* src/flat.css */
@plugin "../../plugin.ts"; /* <- This is now pointing to the wrong file */
```

There are generally two approaches that we can do to solve this:

1. **Handle `@import` flattening inside tailwindcss:** While generally
this would give us more freedom and less dependencies, this would
require some work to get all edge cases right. We need to support
layers/conditional imports and also handle all relative urls for
properties like `background-image`.
2. **Rewrite relative paths as a separate postcss visitor:** The
approach this PR takes is instead to implement a custom postcss plugin
that uses the AST to rewrite relative references inside `@plugin` and
`@content`. This has the benefit of requiring little changes to our
existing APIs. The rule is only enabled for relative references inside
`@plugin` and `@content`, so the surface of this rule is very small.

We can use this plugin inside all three current clients:

- `@tailwindcss/postcss` obviously already uses postcss
- `@tailwindcss/cli` also uses postcss to handle `@import` flattening
- `@tailwindcss/vite` allows us to add custom postcss rules via the CSS
pipeline. There are a few cases that we handle with care (e.g. in vite
you can pass a string to the postcss config which is supposed to load
the config from a file).

To validate the changes, we have added both a list of unit test cases to
the plugin itself as well as verified that all three clients are working
as expected:

- `@tailwindcss/postcss` now has an explicit test for this behavior
- `@tailwindcss/cli` and `@tailwindcss/vite` were manually tested by
updating the vite playground. The CLI was run with `--cwd
playgrounds/vite/ -i ./src/app.css -o foo.css`:
<img width="531" alt="Screenshot 2024-07-29 at 11 35 59"
src="https://github.com/user-attachments/assets/78f0acdc-a46c-4c6c-917a-2916417b1001">
RobinMalfait pushed a commit that referenced this pull request Aug 7, 2024
…`@import`ed files (#14063)

We noticed an issue that happened when handling relative file imports in
the `@plugin` and the upcoming `@content` APIs. The problem arises from
relative files that are inside `@import`ed stylesheets. Take, for
example, the following folder structure:

```css
/* src/index.css */
@import "./dir/index.css";
```
```css
/* src/dir/index.css */
@plugin "../../plugin.ts";
```

It's expected that the path is relative to the CSS file that defined it.
However, right now, we use
[`postcss-import`](https://github.com/postcss/postcss-import) to flatten
the CSS file before running the tailwind build step. This causes these
custom-properties to be inlined in a flat file which removes the
information of which file is being referred:

```css
/* src/flat.css */
@plugin "../../plugin.ts"; /* <- This is now pointing to the wrong file */
```

There are generally two approaches that we can do to solve this:

1. **Handle `@import` flattening inside tailwindcss:** While generally
this would give us more freedom and less dependencies, this would
require some work to get all edge cases right. We need to support
layers/conditional imports and also handle all relative urls for
properties like `background-image`.
2. **Rewrite relative paths as a separate postcss visitor:** The
approach this PR takes is instead to implement a custom postcss plugin
that uses the AST to rewrite relative references inside `@plugin` and
`@content`. This has the benefit of requiring little changes to our
existing APIs. The rule is only enabled for relative references inside
`@plugin` and `@content`, so the surface of this rule is very small.

We can use this plugin inside all three current clients:

- `@tailwindcss/postcss` obviously already uses postcss
- `@tailwindcss/cli` also uses postcss to handle `@import` flattening
- `@tailwindcss/vite` allows us to add custom postcss rules via the CSS
pipeline. There are a few cases that we handle with care (e.g. in vite
you can pass a string to the postcss config which is supposed to load
the config from a file).

To validate the changes, we have added both a list of unit test cases to
the plugin itself as well as verified that all three clients are working
as expected:

- `@tailwindcss/postcss` now has an explicit test for this behavior
- `@tailwindcss/cli` and `@tailwindcss/vite` were manually tested by
updating the vite playground. The CLI was run with `--cwd
playgrounds/vite/ -i ./src/app.css -o foo.css`:
<img width="531" alt="Screenshot 2024-07-29 at 11 35 59"
src="https://github.com/user-attachments/assets/78f0acdc-a46c-4c6c-917a-2916417b1001">
RobinMalfait added a commit that referenced this pull request Aug 7, 2024
This PR is an umbrella PR where we will add support for the new
`@source` directive. This will allow you to add explicit content glob
patterns if you want to look for Tailwind classes in other files that
are not automatically detected yet.

Right now this is an addition to the existing auto content detection
that is automatically enabled in the `@tailwindcss/postcss` and
`@tailwindcss/cli` packages. The `@tailwindcss/vite` package doesn't use
the auto content detection, but uses the module graph instead.

From an API perspective there is not a lot going on. There are only a
few things that you have to know when using the `@source` directive, and
you probably already know the rules:

1. You can use multiple `@source` directives if you want.
2. The `@source` accepts a glob pattern so that you can match multiple
files at once
3. The pattern is relative to the current file you are in
4. The pattern includes all files it is matching, even git ignored files
1. The motivation for this is so that you can explicitly point to a
`node_modules` folder if you want to look at `node_modules` for whatever
reason.
6. Right now we don't support negative globs (starting with a `!`) yet,
that will be available in the near future.

Usage example:

```css
/* ./src/input.css */
@import "tailwindcss";
@source "../laravel/resources/views/**/*.blade.php";
@source "../../packages/monorepo-package/**/*.js";
```

It looks like the PR introduced a lot of changes, but this is a side
effect of all the other plumbing work we had to do to make this work.
For example:

1. We added dedicated integration tests that run on Linux and Windows in
CI (just to make sure that all the `path` logic is correct)
2. We Have to make sure that the glob patterns are always correct even
if you are using `@import` in your CSS and use `@source` in an imported
file. This is because we receive the flattened CSS contents where all
`@import`s are inlined.
3. We have to make sure that we also listen for changes in the files
that match any of these patterns and trigger a rebuild.

PRs:

- [x] #14063
- [x] #14085
- [x] #14079
- [x] #14067
- [x] #14076
- [x] #14080
- [x] #14127
- [x] #14135

Once all the PRs are merged, then this umbrella PR can be merged. 

> [!IMPORTANT]  
> Make sure to merge this without rebasing such that each individual PR
ends up on the main branch.

---------

Co-authored-by: Philipp Spiess <hello@philippspiess.com>
Co-authored-by: Jordan Pittman <jordan@cryptica.me>
Co-authored-by: Adam Wathan <adam.wathan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants