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(css/minifier): Initialize crate #2884

Merged
merged 9 commits into from
Nov 27, 2021
Merged

feat(css/minifier): Initialize crate #2884

merged 9 commits into from
Nov 27, 2021

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Nov 26, 2021

No description provided.

@kdy1 kdy1 added this to the v1.2.113 milestone Nov 26, 2021
@devongovett
Copy link
Contributor

I've also been working on a CSS minifier in Rust. Not sure if you saw it. https://github.com/parcel-bundler/parcel-css I plan to publish a rust crate for it soon. Would be cool to collaborate rather than duplicating efforts if you're interested. 😊

@kdy1
Copy link
Member Author

kdy1 commented Nov 26, 2021

@devongovett Yeah I saw it :) Thanks! Yeah, removing duplicated work is important (that's why I always focused on reusable apis) but I'm not sure at the moment.

@alexander-akait Any thoughts?

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

@devongovett I think we can union here under swc, it will be better to have parser/codegen/minificator/visitor in one repo, there are a lot of work done by swc, so I think it will be better to focus on it here, also vercel want to use swc everywhere and for everything, and for html/markdown/mdx/etc, I don't think moving it under different projects is good idea

@alexander-akait
Copy link
Contributor

@devongovett Also I already point in that we can union here and improve it for ecosystem and avoid creating different projects like we as we had before, which led to inconsistency between different tools/libraries/utils

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

You have probably already felt all these problems in parcel with different tools like babel/postcss/etc, their incompatibility, with different API, different logic, options and etc, I think it is good time to avoid it and focus on one tool

@devongovett
Copy link
Contributor

devongovett commented Nov 26, 2021

I don't think it necessarily all needs to be in one repo. JS and CSS are different languages so there wouldn't be much shared between them anyway. As long as a crate is published that SWC and others can consume it should be fine.

SWC is already duplicating the cssparser and selectors crates used in Servo/Firefox, which are already browser grade parsers. I reused them in parcel-css, and built a property parser and serializer on top. In my opinion it's better to reuse these than rebuild them inside SWC.

The other cool thing about this is that contributions actually go into Firefox too, so we are improving the web for everyone! Yesterday I contributed hwb() color support for example.

@alexander-akait
Copy link
Contributor

It is not only about minifier, it is also AST, codegen, visitor and etc, so minification it only one part of big improvements

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

We don't duplicate nothing, it was written from scratch with new CSS spec and other things, servo parser is not updated with new syntax(es) and have some weird and long time unresolved bugs

@alexander-akait
Copy link
Contributor

servo has js parser too, but we extend it more, also added fancy api and other great things, so I don't see problems with recreating parser and keeping them up to day for our purposes, for example we need nesting selectors for CSS-in-JS, because we want to rewrite some babel parser, with servo it will be impossible, your focus only on minification and autoprefixing, but again, it is only one part of many things

@kdy1
Copy link
Member Author

kdy1 commented Nov 26, 2021

I tried using cssparser and selectors, but they did too much trick for performance so Selector from selectors was not processable.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

Yes, that's why I think it should be here, otherwise we will face the same problems as we were before and we will try to build bridge between incompatibles (different tools)

@devongovett
Copy link
Contributor

cssparser is actually very flexible. It doesn't provide an AST at all, that's entirely up to us. Really, it's more of a tokenizer with traits for hooking into parsing. Nested selectors should be possible for example. There is also a CSS draft spec for that which could be implemented. And if something is out of date, we should contribute back! That benefits the whole web.

@alexander-akait
Copy link
Contributor

In long term we focused on bundler and replace webpack, so we will need and js and ts and css and html and maybe more to create bundler

@devongovett
Copy link
Contributor

That will also be duplicating what we are doing in Parcel FWIW. 😉

@alexander-akait
Copy link
Contributor

Again there are some perf problems, and again, we want to have the same API and have ability to fix problems fast than wait contributors, I don't see any problems with our parser

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

Therefore, I already told you that we can join forces here and leave js there as is and not pull all the old problems again, we have pretty stable js/ts part and it already used in many projects, time to move CSS on new level too

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

I would like to add that this work is doing not only to make another minifier, it will be used in vercel, also we want to migrate stylelint on swc due problems with postcss stylelint/stylelint#5586, replace some babel plugins with CSS-in-JS, and more

@devongovett
Copy link
Contributor

IMO it would be ideal for SWC to focus on JS. I think having separate projects for each language encourages more people to get involved, and keeps each project focused. There's no real reason why Babel and PostCSS should be merged for example. SWC is already quite large and sprawling, and expanding into every possible language feels like scope creep to me. As long as crates are published, integration between tools for each language should be easy.

@alexander-akait
Copy link
Contributor

@devongovett I think opposite, different projects = different developers = inconsistency = slow fixes = compatibility issues, we have already gone through this many times

@alexander-akait
Copy link
Contributor

There's no real reason why Babel and PostCSS should be merged for example

https://github.com/rome/tools why not?

@devongovett
Copy link
Contributor

I guess I don't really see where the inconsistency/incompatibility would be. JS and CSS don't integrate, they are completely separate languages.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

And what is difficult to combine work here, what is the real problem, is that the languages are different? It is not problem, ts and js and jsx/tsx are also different, we have plan to implement mdx, it is also different

@alexander-akait
Copy link
Contributor

swc have good API, which is fairly easy to reuse across languages, why we need duplicate it?

@devongovett
Copy link
Contributor

Yeah I don't think you should implement MDX either. Why not let the MDX project do that? I saw they already have a Rust prototype. It should be separate so the MDX project can actually own the implementation. Not everything needs to be in a single project. That's not very inclusive to new ideas (what's the next MDX?).

@kdy1
Copy link
Member Author

kdy1 commented Nov 26, 2021

Hmm.. SWC stands for speedy web compiler from the start so there was #16
I actually also considered porting some css preprocessors, but I gave up just because it requires too much time.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

Yes, goal is bundling and bundling without problems, if you want to develop own rust based css parser, you can do it, or you can help us, we will be glad too

@devongovett
Copy link
Contributor

Yeah, that's exactly it. Adding every single language to SWC will only increase the amount of work you have to do, not only at first, but in maintenance. Separate projects at reasonable boundaries (eg languages) allows this to scale across the community much better.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

Community also can help us here, it is not problem

@alexander-akait
Copy link
Contributor

I deeply cannot understand why the current job annoys you so, we want to keep it together to provide good DX, avoid problems between api, solve bundling problems, improve perf, and etc, many of them already solved in swc, css is just next step

@devongovett
Copy link
Contributor

devongovett commented Nov 26, 2021

You're also welcome to contribute to Parcel. We have already started Rust based bundling based on SWC earlier this year, and will be migrating more to Rust over time. Would be a shame to duplicate that work just because you want it in the same repo.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

We want to provide abilities not only for parcel, for all community

@alexander-akait
Copy link
Contributor

Thanks for shame me on my work

@devongovett
Copy link
Contributor

You're acting like I'm the one duplicating effort here, but I am really trying to reuse as much as possible from already existing tools, and I am offering to help you do the same. It just seems like you want to reinvent everything yourself. That's fine if you want to, but it isn't great for the community. I don't see the harm in keeping things separate and letting other groups work independently. This will only lead to more new ideas and innovation. If everything is stuck together controlled by one single group of devs, it will be harder to try new ideas and experiment. To me, it's better to let that happen and integrate everything through plugins for example.

@kdy1
Copy link
Member Author

kdy1 commented Nov 26, 2021

Real problem is that, we already have ast/paresr/codegen/visitor for css. I'm fine with using external libray, but using css crates of swc with external minifier would be a pain point

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

I don't see problems with having based web technologies in one place, it will allow to implement good full featured bundler with good perf and yes there will be API for custom goals

@alexander-akait
Copy link
Contributor

We don't want to limit anyone, just provide a basic set of popular solutions out of box

@devongovett
Copy link
Contributor

Yeah I would split it at the language level, not the individual tool (eg minifier) level. Way easier to consume a crate that does all the CSS or HTML stuff than have a parser in one place, a printer in another, etc.

To be clear I love SWC. It's great for JS, and we like using it in Parcel. I'm just not sure it needs to become a monolithic tool with all languages in one. At some point you need to decide where the scope ends so you don't end up with a giant maintenance burden. We went too far with that in Parcel 1, accepting all sorts of languages from contributors into core and then not being able to maintain them all, so I've seen what can happen.

SWC is already quite large in scope for JS alone - way bigger than previous tools which usually focused on a single part (eg transpiling, minifying, bundling, linting, etc). I think combining these parts makes sense because they can share an AST and improve perf. But once you go cross language, those benefits are less.

@kdy1
Copy link
Member Author

kdy1 commented Nov 26, 2021

I'm sorry but I'll read it tomorrow.
I'm too sleepy because I failed to get into sleep yesterday.

@alexander-akait
Copy link
Contributor

We already say we want to have all basic technologies for bundling, CSS is part of bundling, HTML too and some things too, in the current place you constantly indicate that you are doing everything for the parcel, we want to do for ecosystem, so you don't see our goal, you can also reuse our parser/codegen/visitor, that is not problem

@devongovett
Copy link
Contributor

No, there's a reason the CSS compiler is in a separate repo and not in the Parcel monorepo. I plan to expose both a JS and Rust API for CSS that can be used outside Parcel the bundler.

Anyway, it seems like you want to work on your own project. That's fine. I just wanted to see if there was any interest in collaborating on existing tools before you got too far into it.

@alexander-akait
Copy link
Contributor

alexander-akait commented Nov 26, 2021

Just because you don’t want to have it in one place it does not mean that we should deviate from goals.

For performance, consistency API, fast response on problems/issues and more, so we want to keep it in one place. Otherwise, it will become unmanageable and cause a lot of problems. I repeat, if you have a desire, you can continue your work, nobody makes you stop it.

If you want to collaborating, welcome, having lexer and parser is not the problem, many tools reimplement lexers/parsers for own and not only goals, so you can reuse it everywhere. I don't see problems having this together. And I don’t see how it can block forward movement for any.

I did not claim that this is my project or that I only want to work on it. The only reason you answer is that you would like it to be in different repositories, but I don't see any problem with that. We just want to make it better and has no goals to make someone worse, you can always join us and help.

@devongovett
Copy link
Contributor

Yeah it seems like we disagree on goals. I am also using a very different AST format (closer to how browser CSS parsers work), so if we wanted to collaborate either way, we'd need to reconcile that.

@kdy1
Copy link
Member Author

kdy1 commented Nov 27, 2021

I'm back! (from 12 hours of sleep)

I agree that the scope is swc much larger than previous tools. It tries to reimplement various tools in rust and it was an oversight I think. When I started swc, I was even not a CS student. At the moment, I was similar to a student who only graduated high school, so I didn't know how much effort is required to maintain projects.

That's why I targeted swc as a replacement for babel, google closure compiler (not terser at the moment), postcss, autoprefixer, and more. And of course, I faced too many issues to work on, and it became too hard to maintain. So I understand your concerns. It was so true, but not now. It's becoming maintainable again, thanks to contributors. I think it will be more affordable as time goes and more people use swc.

I'm using monorepo to reduce the maintenance burden. If I split the repository, yeah, you are right on some points. If so, I will be able to assign maintainer roles to people. e.g. @alexander-akait contributed a lot to css, and I think he deserves a maintainer role of virtual css repository. It would require splitting other crates like @swc/core, the node binding, and I think it will require more work. But thanks! While writing this, I found that I can make @swc/css npm package which only contains css-related features instead of adding them to @swc/core. Now the problem is bundler... I want to make it able to handle css natively. It requires more thinking, so I'll postpone this.

there was any interest in collaborating on existing tools

I'd love to, but I think it's not possible in this case, mainly because of swc_common::Span. I tried using cssparser, but I gave up because of the span issue. If your crate uses generic for span or some other differences, I'd love to use it, but I think integration with existing swc crates is important than reducing duplicate work.

@kdy1 kdy1 merged commit c6cb790 into swc-project:main Nov 27, 2021
@kdy1 kdy1 deleted the css-opt branch November 27, 2021 05:33
@alexander-akait
Copy link
Contributor

@devongovett

if we wanted to collaborate either way, we'd need to reconcile that

I am open to any ideas, but I agree that the current AST is not the best and I am working on it, also worth mentioning CSS can be parser in two modes - grammar and without (i.e. https://www.w3.org/TR/css-syntax-3/#parsing), and both parsing mode can be useful, for bundling purposes we don't need to apply grammer, even more, lexer can be enough, but for good minification grammar is good. Anyway, I'll be glad if you help, I really don't see any problem that we are going to keep it here.

@devongovett
Copy link
Contributor

Heh, just saw postcss-rs (linked above). I guess there will be a number of these.

@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants