Fix issue around resolving paths in @tailwindcss/vite#19947
Fix issue around resolving paths in @tailwindcss/vite#19947RobinMalfait merged 4 commits intomainfrom
@tailwindcss/vite#19947Conversation
This adds some guards to make sure that if we resolve a value, that: 1. It actually resolved _something_ 2. It's not the same as the `id` we provided 3. It's an absolute path (which we can read from disk) If that's not the case, then we end up with `undefined`, which means that the `@tailwindcss/node` resolver will kick in and try to resolve that way. This should make it compatible with Vite projects that have an `@` alias setup.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds Vite and SvelteKit integration tests to validate Tailwind CSS alias resolution and plugin package loading: two tests in 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
integrations/vite/resolvers.test.ts (1)
167-289: Add a Vite 5 regression test for package plugins with at-sign aliases.The new package-plugin regressions only test
vite: "^8". However,packages/@tailwindcss-vite/src/index.tscontains a separate back-compat resolver path for older Vite APIs (lines 51-53: "Older, pre-environment Vite API"; lines 93-119:createBackCompatIdResolverwith comment "Remove this function and pre-environment code when Vite < 7 is no longer supported"). Adding a Vite 5-style regression combining package plugins and@-aliases would prevent that compatibility branch from drifting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations/vite/resolvers.test.ts` around lines 167 - 289, Add a Vite 5 regression test variant that exercises the back-compat resolver path by duplicating the "resolves package plugins in production build with at-sign aliases" test but using "vite": "^5" in its devDependencies (so the test hits the older pre-environment API and createBackCompatIdResolver code path in packages/@tailwindcss-vite/src/index.ts); keep the same fixtures (index.html, src/index.css with `@plugin` entries, and vite.config.ts that defines the "@" alias) and the same assertions against the built CSS to ensure package plugins + `@-aliases` work under Vite 5.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/vite/sveltekit.test.ts`:
- Around line 75-78: The test currently assumes the first matched CSS asset
contains the class by using files[0][0]; change the assertion to scan all
results from fs.glob (the files variable) and assert that at least one of the
matched file paths passed to fs.expectFileToContain contains the
candidate`form-input` token. In other words, iterate over files (from fs.glob),
call or check each path with fs.expectFileToContain (or read and check contents)
and assert that some file includes candidate`form-input`, rather than relying on
files[0][0].
---
Nitpick comments:
In `@integrations/vite/resolvers.test.ts`:
- Around line 167-289: Add a Vite 5 regression test variant that exercises the
back-compat resolver path by duplicating the "resolves package plugins in
production build with at-sign aliases" test but using "vite": "^5" in its
devDependencies (so the test hits the older pre-environment API and
createBackCompatIdResolver code path in
packages/@tailwindcss-vite/src/index.ts); keep the same fixtures (index.html,
src/index.css with `@plugin` entries, and vite.config.ts that defines the "@"
alias) and the same assertions against the built CSS to ensure package plugins +
`@-aliases` work under Vite 5.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 43e57313-71c8-435a-bf7c-683867e18b4e
📒 Files selected for processing (4)
integrations/vite/resolvers.test.tsintegrations/vite/sveltekit.test.tspackages/@tailwindcss-postcss/README.mdpackages/@tailwindcss-vite/src/index.ts
💤 Files with no reviewable changes (1)
- packages/@tailwindcss-postcss/README.md
| let files = await fs.glob('.svelte-kit/output/client/**/*.css') | ||
| expect(files.length).toBeGreaterThan(0) | ||
|
|
||
| await fs.expectFileToContain(files[0][0], [candidate`form-input`]) |
There was a problem hiding this comment.
Don't rely on the first emitted CSS asset.
SvelteKit can emit multiple client CSS files, and files[0][0] makes this regression depend on glob ordering. Scan all matched CSS assets and assert that at least one contains candidate\form-input`` instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@integrations/vite/sveltekit.test.ts` around lines 75 - 78, The test currently
assumes the first matched CSS asset contains the class by using files[0][0];
change the assertion to scan all results from fs.glob (the files variable) and
assert that at least one of the matched file paths passed to
fs.expectFileToContain contains the candidate`form-input` token. In other words,
iterate over files (from fs.glob), call or check each path with
fs.expectFileToContain (or read and check contents) and assert that some file
includes candidate`form-input`, rather than relying on files[0][0].
RobinMalfait
force-pushed
the
fix/issue-19946
branch
from
186f2d5 to
87961c4
Compare
tailwind fixed the bug using this website as their test project! tailwindlabs/tailwindcss#19947 so now we can update to the latest tailwind. also, minor stylistic change in the Containerfile as this article says to run using `node build` https://svelte.dev/docs/kit/adapter-node#Deploying pretty sure it's basically the exact same thing but let's use their version :)
1 participant
This PR fixes an issue when using
@tailwindcss/viteand you're trying to resolve paths. In the latest 4.2.3 release, we added support for following the vitealiasesoption.However, some people run into issues because if you just use
@as an alias then using@tailwindcss/typographywouldn't resolve because it's a package, and not something local.With this PR we fix that by making sure that Vite's resolver can actually resolve to an absolute path. If not, then we fallback to the resolving that happens in
@tailwindcss/node.Fixes: #19946
Test plan
Before:
After:
