-
Notifications
You must be signed in to change notification settings - Fork 641
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
feat(tailwind): Smaller bundle size #1383
base: canary
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -28,7 +28,6 @@ | |||
"pnpm": { | |||
"patchedDependencies": { | |||
"postcss-css-variables@0.19.0": "patches/postcss-css-variables@0.19.0.patch", | |||
"process@0.11.10": "patches/process@0.11.10.patch", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package was one that we used as a polyfill that isn't needed and isn't required to be patched anymore!
}; | ||
|
||
const nextAppLocation = path.resolve(__dirname, "./nextjs"); | ||
$("npm install", nextAppLocation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried using pnpm
here, which would be better for sure, but, for some reason, the preview deploy of our app at https://react.email failed because of it. Once I moved to using npm
this worked without errors.
return processElement(renderedComponent); | ||
if (hasNonInlineStylesToApply) { | ||
let hasAppliedNonInlineStyles = false as boolean; | ||
mappedChildren = mapReactTree(mappedChildren, (node) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the same idea from what we merged recently that allows the <head>
element to be at any depth, this maps the entire React tree to find it.
@@ -22,6 +22,34 @@ describe("Tailwind component", () => { | |||
}); | |||
}); | |||
|
|||
// test("with React context and custom components", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A unit test I kept commented out as useful for when we get to #1104.
Co-authored-by: Benny Burrito <87976514+bennyburrito@users.noreply.github.com> Co-authored-by: Ben Verspeak <ben.verspeak@yonderland.com> Co-authored-by: Gabriel Miranda <gabrielmfern@outlook.com>
… is not installed (#1387) Co-authored-by: zhanghong <hong.zhang@axinan.com>
…e of the Tailwind component
…enerate styles for class names
…ities instead of regexes
This function is meant to do the sanitization that we need for the best email client support. As of this commit, it only sanitizes the `rgb` calls it finds.
…name of their digit This is because the only way to have these numbers in class names for CSS is to either escape them with something like "\3". But the issue with that is that we can't really have escape characters in class selectors as they aren't very well supported.
…nts in the React tree
d079aa6
to
364d55b
Compare
4473810
to
ad2e6c4
Compare
4ec6fb3
to
b9fc28a
Compare
Background
After the updates to the Tailwind component that came with performance improvements
we had a lot more environments to consider while trying to run
tailwindcss
under the hood for generating the styles from classes.
These environment constraints, like being able to run on the browser and on other environments,
forced us into having to polyfill certain node modules — that both
postcss
andtailwindcss
use under the hood — while bundling even though these code paths were never really
hit when running.
The reason these code paths were not completely tree shaken out of the built code was because
we called
tailwindcss
directly as a plugin onpostcss
.Why?
Polyfilling was the temporary solution for this, but it is not optimal in the least. This caused
issues like:
next-intl
's createTranslator with Tailwind component #1244And ended up also being a burden to maintain and debug.
The solution
The best way I've found to solve these issues is by calling out the internal Tailwind functions
that do what they need to do on
postcss
data structures. This also allows us to substitute a bit ofour logic, which was previously purely done with regexes, with
postcss
's utilities which makes it much easier to readand maintain.
The reason I choose to call out the internal Tailwind functions was because the code that was Node specific
was really just around finding the configuration or checking if a certain thing was installed, and
this would really only run when it was called as a
postcss
plugin.The way I do call out the internal Tailwind functions is heavily inspired by the way they do it
themselves on the Tailwind VS Code extension/LSP Server.
Other changes that make this a refactor
Along with this, I also had to change up a few things. The first and biggest one was applying
the non-inlinable styles — currently media queries, i.e.
sm:...
— after our first passon the React tree which is for inlining styles. This is because we are now generating the styles
for each element's classes on-the-fly instead of at once (which is still performant the way it is
implemented), so we need to aggregate all the non-inlinable classes and their CSS to add to the
<head>
later.
The second change should be mostly internal, (and should also make it easier for others to contribute),
which is a new
mapReactTree
utility that goes through the whole tree and separates the concernsabout when to go recursive on the elements or not. It will also make it easier to actually look at #1104 once
we get there.
Added tests for ensured stability
As I mentioned, we had to consider a bunch of different environments and testing them manually
was really time-consuming, so, borrowed from #1124, I implemented an integration tested powered by yalc that allows us to make sure that the component will work well with NextJS's build process.
You can find this test inside
packages/tailwind/integrations/_tests/nextjs.spec.ts
. This PR also adds a test forVite
as well.