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

feat!: bun css support #461

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

feat!: bun css support #461

wants to merge 3 commits into from

Conversation

nobkd
Copy link
Collaborator

@nobkd nobkd commented Jan 26, 2025

No description provided.

@nobkd nobkd requested a review from tipiirai January 27, 2025 01:35
@nobkd
Copy link
Collaborator Author

nobkd commented Jan 27, 2025

We could also make the bun css option non-default first, and change that, if we find it stable. I have just played around with it, and it didn't show any issues, but maybe there are some.

I'm also still looking, if I can somehow replace the external array with an easier alternative, that marks css as internal and everything else as external...

@tipiirai
Copy link
Contributor

OH! I have completely missed this. Lets briefly talk before merging. So what exactly is Bun CSS support? Does this mean we can get rid of the heavy Lightning CSS support at some point? Now even? I would love to ditch LCSS because it makes 90% of all dependenices megabyte wise.

@nobkd
Copy link
Collaborator Author

nobkd commented Jan 27, 2025

It's basically a rewrite of lightningcss in Zig by the Bun team, and it's included in Bun itself.

Yes, we could get rid of lightningcss (on bun).

Bun css is not as configurable, but I think the defaults set currently are probably alright.

Node should still work, if lcss gets installed locally. (like esbuild)

I currently added a cli option, that switches to lightningcss. There's probably still a bit of documentation, that needs to get updated.

I still have some issues with the current implementation (e.g. the external array).
I'll try taking a stab at them later today.

@nobkd nobkd marked this pull request as draft January 27, 2025 13:35
@nobkd nobkd changed the title feat: bun css support feat!: bun css support Jan 27, 2025
@tipiirai
Copy link
Contributor

Hard to find any information about this. What happens with nested selectors? Are modern color functions supported? How the minifier works? etc. I really want this to replace Lightning CSS npm requirement. Would make Nue exceptionally slim. Developers could potentially enable LCSS support with local npm install.

@nobkd
Copy link
Collaborator Author

nobkd commented Jan 29, 2025

As I said, Bun CSS is basically a rewrite of lightningcss:
https://github.com/oven-sh/bun/blob/93a89e5866fa85fea4026b084ec3b6efd1b6bdc0/src/css/README.md

I think this is the basic browser target, but I'm not sure:
https://github.com/oven-sh/bun/blob/93a89e5866fa85fea4026b084ec3b6efd1b6bdc0/src/css/targets.zig#L149-L154
To support those, nesting is currently unwrapped I guess.
I'm not sure, what happens with the modern color functions though.

I haven't seen any option to enable / disable any feature flags on Bun.build sadly. (even though it seems possible in the zig code)
https://github.com/oven-sh/bun/blob/93a89e5866fa85fea4026b084ec3b6efd1b6bdc0/src/css/targets.zig#L12-L18
https://github.com/oven-sh/bun/blob/93a89e5866fa85fea4026b084ec3b6efd1b6bdc0/src/css/targets.zig#L116-L136

Note: I dind't get the css bundler to properly work with versions of Bun <1.2, so I changed the engines. I hope to still get it to work, but I can't really promise anything.

@tipiirai
Copy link
Contributor

Okay, wow. Looking at the Bun code they seem to be taking CSS seriously. A major Zig engineering project. Happy to merge this and Bun 1.2 requirement is not an issue. I would even depreciate Node for this — not a fan of branching to ESBuild and Lightning CSS just to get Node working. Bun is such a stable product these days. Nue website CSS is a good test framework for this feature: if all looks good, then we're good to go. Thoughts?

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