Skip to content

chore: upgrade tailwind v4 #784

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

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

DennisSmuda
Copy link

Description

Upgrade Tailwind to v4.

Additional context

Has a couple of breaking changes that need some attention

https://tailwindcss.com/docs/upgrade-guide


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Dependency Upgrade

Copy link

stackblitz bot commented Feb 25, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

codesandbox bot commented Feb 25, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

netlify bot commented Feb 25, 2025

Deploy Preview for histoire-site failed.

Name Link
🔨 Latest commit 4ca4ed1
🔍 Latest deploy log https://app.netlify.com/sites/histoire-site/deploys/67dc6bf4344fe30007a60fd0

Copy link

netlify bot commented Feb 25, 2025

Deploy Preview for histoire-controls failed.

Name Link
🔨 Latest commit 4ca4ed1
🔍 Latest deploy log https://app.netlify.com/sites/histoire-controls/deploys/67dc6bf45c21c90008d0ee7c

Copy link

netlify bot commented Feb 25, 2025

Deploy Preview for histoire-examples-svelte3 failed.

Name Link
🔨 Latest commit 4ca4ed1
🔍 Latest deploy log https://app.netlify.com/sites/histoire-examples-svelte3/deploys/67dc6bf4affe480008b7c883

Copy link

netlify bot commented Feb 25, 2025

Deploy Preview for histoire-examples-vue3 failed.

Name Link
🔨 Latest commit 4ca4ed1
🔍 Latest deploy log https://app.netlify.com/sites/histoire-examples-vue3/deploys/67dc6bf4b0d380000823f695

@hugoattal
Copy link
Collaborator

Hey Dennis, thank you so much for your contribution 😄 ! It seems some tests are failing, so we're waiting for everything to work properly before reviewing 😉 (don't mind the svelte test)

If you need any help, feel free to ask!

@DennisSmuda
Copy link
Author

Hey Dennis, thank you so much for your contribution 😄 ! It seems some tests are failing, so we're waiting for everything to work properly before reviewing 😉 (don't mind the svelte test)

If you need any help, feel free to ask!

Thank you! 🫶

Of course this turned out to be more of a hassle than I initially thought. I'm trying to get each app inside packages to run+build separately first.

histoire-controls-stories is already kinda working 😄

I will let you know how my progress is this week 👍

@DennisSmuda
Copy link
Author

DennisSmuda commented Mar 10, 2025

grafik

@hugoattal this is what it currently looks like :)

I'm still trying to port over the config to the css-based approach - the grays are giving me a hard time, I'm definitely gonna need someone to review the config-logic once I'm done 👍

@DennisSmuda
Copy link
Author

grafik @hugoattal this is what it currently looks like :)

I'm still trying to port over the config to the css-based approach - the grays are giving me a hard time, I'm definitely gonna need someone to review the config-logic once I'm done 👍

I think I'm close to done on the core now - it looks/feels pretty much the same, but I probably missed some things.

I'll get started on the plugins next, the tests are passing locally so far ✅

@DennisSmuda
Copy link
Author

image

Looks good so far - I think the tailwind tokens part is not working correctly anymore, since tailwind doesn't export it's tokens through javascript like they did in v3. I'll look into it, but I checked a couple cypress tests, and they're also passing.

@hugoattal
Copy link
Collaborator

You rock 🤟 ! Let me know if you manage to make the tokens work!

Copy link

pkg-pr-new bot commented Mar 15, 2025

Open in Stackblitz

histoire

npm i https://pkg.pr.new/histoire-dev/histoire@784

@histoire/app

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/app@784

@histoire/controls

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/controls@784

@histoire/plugin-nuxt

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/plugin-nuxt@784

@histoire/plugin-percy

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/plugin-percy@784

@histoire/plugin-screenshot

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/plugin-screenshot@784

@histoire/plugin-svelte

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/plugin-svelte@784

@histoire/plugin-vue

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/plugin-vue@784

@histoire/shared

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/shared@784

@histoire/vendors

npm i https://pkg.pr.new/histoire-dev/histoire/@histoire/vendors@784

commit: 10a602c

@DennisSmuda
Copy link
Author

@hugoattal unfortunately, I don't think there is a way to actually do this right now 😢

Without the ability to get the users' actual config through javascript, we can't really display "their design system" inside the story. We could only "hardcode" the actual stories and variants, but I don't think that would be wise, since it wouldn't account for custom namings/scales the user would have in their repo.

The only thing that ended up working was the borderRadius story - probably only, because I still use the JS-configs and haven't ported everything to CSS-style-config.

I'm not sure how to proceed tbh - I've found a relevant discussion on the tailwind repo here.
Seems like others are having the same struggles.

Screenshot 2025-03-20 at 20 27 13

Let me know how we should proceed, other than this roadbump I'm pretty happy with the upgrade so far 👍

@hugoattal
Copy link
Collaborator

Hum, do you think it would be possible to list the tokens using something like getComputedStyle?

Histoire would lose a big advantage for Tailwind users if we drop this completely 😕...

If there are no solutions, I guess you could disable the tests related to tailwind tokens. Maybe we'll reintroduce them later, if they ever ship an alternative to the resolveConfig function.

@DennisSmuda
Copy link
Author

Hum, do you think it would be possible to list the tokens using something like getComputedStyle?

Histoire would lose a big advantage for Tailwind users if we drop this completely 😕...

If there are no solutions, I guess you could disable the tests related to tailwind tokens. Maybe we'll reintroduce them later, if they ever ship an alternative to the resolveConfig function.

I agree that it would be a downgrade!

You might be right, the official docs also suggest doing getComputedStyle(document.documentElement) - but in tailwind-tokens.ts we are inside a node script.

So (if I understand correctly) we can't really get the css-vars that are on root/document at this point in the code.

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