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(compiler): static branching to handle conditional expressions #229
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 23fe484:
|
Hey, this looks cool ! I noticed this PR after @poteboy 's tweet, since we have a similar feature in Panda I can maybe provide a few tips
I don't think it'll be much slower unless the
indeed, for that I recommend having an online playground and/or a
fwiw, we use
we do this here:
then we have this
regarding btw, now that I'm thinking about it, you could very well use the we use it though the extract fn (also exposed from |
@astahmer thanks for the comment! Somehow I haven't looked at Panda's implementation yet and I only checked other zero-runtime libs such as linaria or atlassian/compiled. Just to provide some more context, around 2021, I applied similiar techniques together with Linaria-like evaluator to Indeed's web sites. This information is public:
From Speed Matters, But It Isn’t Everything - Indeed Engineering Blog There I created generic optimizer which can even completely compile out Chakra UI's Box since it runs SSR-like logic during the build time to extract style tags generated from emotion. ts-evaluator is cool! I did not know that lib. It looks like it's using actual exeuction so I suspect that it can be slower than babel's one although it can be much more powerful? Related to this, do you think ts-morph is better than babel for this kind of logic? Maybe you chose ts-morph to use ts-evaluator? Thanks for providing links to Panda's corresponding implementations, I'll take a look later! For |
@astahmer I played with Panda finally and realized that it's so smart! I really wanted to have this 2 years ago 😅 |
this sounds very interesting ! would be cool if you have the time to opensource it 🙏
I can't say for sure as I haven't dug into the library's code too deeply but yes I think so, since we can pass an "environment" with variables/functions etc it can be pretty powerful
(I've been asked this question before, I'm pasting from this tweet): I havent used much babel for that, but what I like regardless with ts-morph is:
|
yes this one is something that ideally should be supported but this would require a bit of work on the so definitely not high in the issue priority since we have the we covered that in the docs here |
Note: we discussed with core maintainers and decided to go with slightly different way and continue in #233 For now we avoid relying on external dependencies such as packages from Panda CSS. We might reconsider though. Thanks for all the comments and pointers! Closing this PR |
Note: this PR might not be what Kuma UI core devs want to have (See "Tradeoffs" section). In this case, feel free to just reject it! It was just fun to explore the code base for my learning purpose
This PR implements a new way to make more patterns "static".
During the build time, the following:
becomes
with static CSS output. There is a limitation for the maximum number of combinations (hardcoded as
8
in this PR) but practically we should not have too many flags in styles anyway.extractAttribute
can return UnevaluatedConditionalExpression, which holds the conditional expression, whenTrue, and whenFalseextractProps
callsevaluateStaticBranching
in a newly addedstatic-branching.ts
to statically branch the potential paths. For each possibility, it will generate CSS along with "index expression", which will be used to choose actual className in runtime.updateClassNameAttr
to be used in the final stage after the static branchingstatic-branching.ts
but not sure whether this is the right wayNote that I did not modify extractPseudoAttribute for now. I think it's better to start small rather than modifying too many places until this gets proven to be actually useful
Tradeoffs
This strategy has several benefits and drawbacks.
I would like Kuma UI core devs to think about these trade offs before deciding whether we should merge this PR or not.
Benefits
flag ? 'bg-red' : 'bg-blue'
) without adding performance cost. People can still say "CSS-in-JS" is slow just due to this pattern since it is common pattern.Drawbacks
static-branching.ts
with unit tests)Alternatives considered
As an alternative, we could generate multiple static classNames for each conditional, however in this PR I did not choose this path simply because the code will be more complicated. I believe that practically generated styles will not be so large especially with CSS optimizers and the runtime performance matters more. This way can be easier if the overall design is based on "atomic styles" like Linaria does in callstack/linaria#867
Future potential improvements
Here is the list of things I did not implement in this PR to avoid too much change at this point:
static-branching.ts
can be further improved like recognizing "actually the same flag" such as!flag
andflag
. This kind of improvement should not add much maintenance cost since it's completely encapsulated instatic-branching.ts
{margin: flag ? 1 : 2}
. This will be especially useful for pseudo props such as_hover
We might want to do further refactor if we go with this path. For example, probably it's better to reconsider the usage of ts-morph in favor of babel if there is no actual plan to utilize type information. Babel has very useful utilities such as
nodePath.evaluate
(StackBlitz example)Manual testing with Next.js
I tested this change with Next.js using the following code:
Before this PR, this falled back to "dynamic mode" and without
KumaRegistry
, it was causing FOUC.After this PR, this generates two class names with styles statically and there is no FOUC since now it's "static" (almost exactly the same performance as Tailwind / manual CSS except for the overhead of
Box
itself, which is different thing we could optimize out in the future)Misc: How other libraries are doing?
TODO: This might be kind of novel idea (similar idea originally coming from my closed source prototype several years ago) but not sure. I suspect some popular libs might have similar logic, let me know if anyone find such code since I am interested!Things to care if we decide to ship this PR