Fix Windows tests for new postcss plugin #14085
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philipp-spiess
force-pushed
the
fix/windows-tests-for-postcss-plugin
branch
from
d03f9b7 to
8c0bbb8
Compare
philipp-spiess
force-pushed
the
feat/add-content-support
branch
from
6d39204 to
b078327
Compare
philipp-spiess
force-pushed
the
fix/windows-tests-for-postcss-plugin
branch
from
8af6800 to
d6cc19c
Compare
philipp-spiess
force-pushed
the
fix/windows-tests-for-postcss-plugin
branch
from
d6cc19c to
86de233
Compare
RobinMalfait
approved these changes
Jul 30, 2024
There was a problem hiding this comment.
All the Windows paths make me nervous, but was on a call with @philipp-spiess so going to approve already.
I think it's good to get at least @thecrypticace eyes on it before merging.
|
|
||
| function fixRelativePath(atRule: AtRule) { | ||
| let rootPath = getRoot(atRule)?.source?.input.file | ||
| let rootPath = atRule.root().source?.input.file |
There was a problem hiding this comment.
I simplified this. Node has a root() method that walks up the tree for you.
It also stops at Root instead of a Document (an optional parent node of Root)
| } | ||
|
|
||
| let inputFilePath = atRule?.source?.input.file | ||
| let inputFilePath = atRule.source?.input.file |
There was a problem hiding this comment.
atRule isn't nullable so one of the ? could be removed
| @@ -1,5 +1,6 @@ | |||
| import path from 'node:path' | |||
| import type { AtRule, Plugin } from 'postcss' | |||
| import { normalizePath } from './normalize-path' | |||
There was a problem hiding this comment.
I inlined a corrected version of the normalize-path NPM package (specifically regarding network shares and UNC paths) and used that method to cleanup the code slightly.
thecrypticace
approved these changes
Jul 30, 2024
philipp-spiess
added a commit
that referenced
this pull request
Aug 2, 2024
This PR fixes the new `postcss-fix-relative-paths` plugin for Windows paths. The issue was that we mixed Windows-style path separators from the absolute file paths with the Posix-style separators from globs. This caused the `dirname` functions to resolve to the wrong files. To solve this, we now make the difference very clear by calling the content a `glob`. For globs, we always expect Posix-style path separators and for the case of making a glob absolute (by prefixing the directory), we now convert them into Posix-style explicitly. This PR also fixes an issue where negative rules (e.g. `!./**/*.ts`) were not properly rewritten. --------- Co-authored-by: Jordan Pittman <[email protected]>
philipp-spiess
added a commit
that referenced
this pull request
Aug 5, 2024
This PR fixes the new `postcss-fix-relative-paths` plugin for Windows paths. The issue was that we mixed Windows-style path separators from the absolute file paths with the Posix-style separators from globs. This caused the `dirname` functions to resolve to the wrong files. To solve this, we now make the difference very clear by calling the content a `glob`. For globs, we always expect Posix-style path separators and for the case of making a glob absolute (by prefixing the directory), we now convert them into Posix-style explicitly. This PR also fixes an issue where negative rules (e.g. `!./**/*.ts`) were not properly rewritten. --------- Co-authored-by: Jordan Pittman <[email protected]>
RobinMalfait
pushed a commit
that referenced
this pull request
Aug 7, 2024
This PR fixes the new `postcss-fix-relative-paths` plugin for Windows paths. The issue was that we mixed Windows-style path separators from the absolute file paths with the Posix-style separators from globs. This caused the `dirname` functions to resolve to the wrong files. To solve this, we now make the difference very clear by calling the content a `glob`. For globs, we always expect Posix-style path separators and for the case of making a glob absolute (by prefixing the directory), we now convert them into Posix-style explicitly. This PR also fixes an issue where negative rules (e.g. `!./**/*.ts`) were not properly rewritten. --------- Co-authored-by: Jordan Pittman <[email protected]>
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 <[email protected]> Co-authored-by: Jordan Pittman <[email protected]> Co-authored-by: Adam Wathan <[email protected]>
mark-tomlinson-dev
pushed a commit
to sugarcube-org/scanner-temp
that referenced
this pull request
Aug 19, 2025
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] tailwindlabs/tailwindcss#14063 - [x] tailwindlabs/tailwindcss#14085 - [x] tailwindlabs/tailwindcss#14079 - [x] tailwindlabs/tailwindcss#14067 - [x] tailwindlabs/tailwindcss#14076 - [x] tailwindlabs/tailwindcss#14080 - [x] tailwindlabs/tailwindcss#14127 - [x] tailwindlabs/tailwindcss#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 <[email protected]> Co-authored-by: Jordan Pittman <[email protected]> Co-authored-by: Adam Wathan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the new
postcss-fix-relative-pathsplugin for Windows paths. The issue was that we mixed Windows-style path separators from the absolute file paths with the Posix-style separators from globs. This caused thedirnamefunctions to resolve to the wrong files.To solve this, we now make the difference very clear by calling the content a
glob. For globs, we always expect Posix-style path separators and for the case of making a glob absolute (by prefixing the directory), we now convert them into Posix-style explicitly.This PR also fixes an issue where negative rules (e.g.
!./**/*.ts) were not properly rewritten.