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

Revert: Only expose used CSS variables #16403

Merged
merged 4 commits into from
Feb 10, 2025

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented Feb 10, 2025

This reverts #16211

We found some unexpected interactions with using @apply and CSS variables in multi-root setups like CSS modules or Vue inline <style> blocks that were broken due to that change. We plan to re-enable this soon and include a proper fix for those scenarios.

Test plan

  • Updated snapshots
  • Tested using the CLI in a new project:
    Screenshot 2025-02-10 at 13 08 42

@philipp-spiess philipp-spiess marked this pull request as ready for review February 10, 2025 11:53
@philipp-spiess philipp-spiess requested a review from a team as a code owner February 10, 2025 11:53
@@ -0,0 +1 @@
export const enableRemoveUnusedThemeVariables = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could set this conditionally such that local tests don't require any changes. But I think this is fine for now 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I tried this like I had in the feature flag test PR but I couldn’t get the local build right. Wanted to be able to build a production release to be sure it works

@RobinMalfait RobinMalfait enabled auto-merge (squash) February 10, 2025 12:26
@RobinMalfait RobinMalfait merged commit 9bbe2e3 into main Feb 10, 2025
5 checks passed
@RobinMalfait RobinMalfait deleted the fix/revert-remove-unused-theme-variables branch February 10, 2025 12:26
@The-Best-Codes
Copy link

🥲 I was so excited about that update…

@philipp-spiess
Copy link
Member Author

@The-Best-Codes Plan is to fix the issues very soon so we can re-land this soon. Just need to do this without breaking a lot of setups that rely on CSS modules or things like <style> blocks in Vue and Svelte files. Sorry!

@The-Best-Codes
Copy link

No need to apologize. 🙂
I care about package stability a lot too, which Tailwind is doing an excellent job of maintaining. ❤️

peterpeterparker added a commit to dfinity/oisy-wallet that referenced this pull request Feb 18, 2025
# Motivation

We detected reproducibility issue when building v0.24.0:

https://github.com/dfinity/oisy-wallet/actions/runs/13393064153

~~I'm unsure which of the two following Tailwind fix was the reason of
the problem but, bumping Tailwind to latest version seems to resolve the
problem.~~

~~Tailwind v4.0.6 - Fix
[#16403](tailwindlabs/tailwindcss#16403
~~- Revert change to no longer include theme variables that aren't used
in compiled CSS ~~

~~Tailwind v4.0.7 - Fix
[#16414](tailwindlabs/tailwindcss#16414
~~- Fix sorting of numeric utility suggestions when they have different
magnitudes~~

~~I lean towards to former though. I could have try v4.0.6 but, v4.0.7
contains interesting fixes anyway.~~

It does not resolve the issue:
https://github.com/dfinity/oisy-wallet/actions/runs/13395505962/job/37413394328



# Changes

- Bump `@tailwindcss/postcss`, `@tailwindcss/vite` and `tailwindcss`.
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.

3 participants