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

Upgrade tailwind version #6550

Merged
merged 15 commits into from
Jan 18, 2024

Conversation

prajwalkulkarni
Copy link
Contributor

@prajwalkulkarni prajwalkulkarni commented Jan 13, 2024

💫 Changelog

⭐ Upgrade tailwindcss to 3.4.1.
⭐ Adds support for min-width min-w-* out of the box and more (https://tailwindcss.com/blog/tailwindcss-v3-4)
⭐ Removed 80: '20rem' from minWidth in tailwind.config.js

Copy link

github-actions bot commented Jan 13, 2024

Size changes

📦 Next.js Bundle Analysis for react-dev

This analysis was generated by the Next.js Bundle Analysis action. 🤖

🎉 Global Bundle Size Decreased

Page Size (compressed)
global 103.86 KB (🟢 -49 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Five Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/404 78.04 KB (🟡 +25 B) 181.91 KB
/500 78.04 KB (🟡 +25 B) 181.9 KB
/[[...markdownPath]] 79.63 KB (🟡 +25 B) 183.5 KB
/errors 78.22 KB (🟡 +25 B) 182.09 KB
/errors/[errorCode] 78.2 KB (🟡 +25 B) 182.06 KB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 10% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

@prajwalkulkarni
Copy link
Contributor Author

Hey Ricky, from our previous conversation (#6519 ), it was decided to upgrade the tailwindcss version to 3.4 as the latest version now supports min-w-*.
I've also removed overwriting the property of minWidth object. Did a quick sanity after the upgrade and everything looks good.

In tailwind.config.js we're overwriting the value of xs in maxWidth to 21rem in contrast to the actual value of 20rem. Although I'm not sure why that was changed, should we consider removing it, so that the default value of 20rem is used?
@rickhanlonii

@@ -60,9 +60,8 @@ module.exports = {
...defaultTheme.maxWidth,
xs: '21rem',
},
minWidth:{
minWidth: {
Copy link
Member

Choose a reason for hiding this comment

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

You can just remove this whole key since there are no overrides anymore.

@rickhanlonii
Copy link
Member

rickhanlonii commented Jan 13, 2024

@prajwalkulkarni not sure why it's 21 and not 20. You could either just leave it, or test every place it's used to see if 20 works just as well (i suspect it won't), or rename it to max-w-custom-xs, so max-w-xs is the default value. I don't have a strong opinion either way.

@prajwalkulkarni
Copy link
Contributor Author

The max-w-xs is mostly used in viewports with breakpoint lg and above. Using 20rem instead of 21rem is causing spacing issues, which requires me to update the width of the parent container as well. So, for the time being, I feel it is ok to keep it as is, although I've updated the name to custom-xs so that the original value corresponding to max-w-xs is preserved.
I will work on updating the widths some other time, maybe as a separate PR so that we can completely get rid of overriding maxWidth, but for this PR, I think we can limit the scope to just upgrading the tailwind version. What do you think?

@rickhanlonii rickhanlonii merged commit 9c316b2 into reactjs:main Jan 18, 2024
4 checks passed
@rickhanlonii
Copy link
Member

Awesome, thanks! In future PRs, please turn off the class sorting because it makes it difficult to review the changes.

@prajwalkulkarni prajwalkulkarni deleted the upgrade-tailwind-version branch January 22, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants