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

Support for css #950

Closed
wants to merge 129 commits into from
Closed

Support for css #950

wants to merge 129 commits into from

Conversation

kdy1
Copy link
Member

@kdy1 kdy1 commented Aug 9, 2020

Closes #16

@pickfire
Copy link
Contributor

@kdy1 Are you working on this? I am helping out with https://github.com/connorskees/grass which is roughly the same speed as libsass (going to be deprecated I heard) and 2x faster than the defacto dart-sass without speed in mind, currently the project only aim to pass as much tests as possible. I wonder if we could use it here or are you planning on writing one?

It passes 3375 of 5093 tests as of 2020-08-07. CC @connorskees

@kdy1
Copy link
Member Author

kdy1 commented Aug 10, 2020

@pickfire I didn't started seriously. Although I will determine after looking it's code, I think we can use it.

@connorskees
Copy link

I'm excited to see this is being discussed! grass integration with swc was actually part of my end-term goals related to evangelizing grass. I would love to see a Webpack written purely in Rust, and Sass support is certainly a big step towards that. Anything that can further the integration between grass and swc will receive complete, in-tree support.

With regard to #16, it is already planned that grass will do automatic minification (in Sass this is referred to as compressed output). In addition, it is my hope that grass will be able to provide an API similar to that of postcss, which will easily enable autoprefixing and cssnext-like functionality.

libsass as well as node-sass and sassc are being officially deprecated (see libsass/3123, node-sass/2952, and sassc/273). What this means for grass, I think, is that we will be essentially on the same footing as libsass when it comes to being a "blessed" implementation. grass has very recently landed an MVP implementation of the module system, and so it is already well ahead of libsass in that regard.

There are a number of known, outstanding issues, but in practice it appears they do not come up very often. grass is currently able to compile a number of Sass libraries including Bootstrap 4, bulma-scss, and materialize. Having said that, I want to be clear that grass is still several months out from being what I would consider production ready. There is currently no support for the indented syntax or @forward, and limited support for @media, @at-root, and min(...)/max(...). There has also not been done any fuzzing for longer than an hour.

Please feel free to ping me for anything related to grass integration with swc or just Sass in general!

@kdy1
Copy link
Member Author

kdy1 commented Aug 10, 2020

@connorskees Great to hear that! I will use it.
Although I'm not sure how should I integrate css loader with swc_bundler, I think it will be way easier with grass.

@pickfire
Copy link
Contributor

pickfire commented Aug 11, 2020

Currently we have two interfaces, I personally think it is not that good. CC @connorskees

pub fn from_path(p: &str, options: &Options) -> Result<String>;
pub fn from_string(p: String, options: &Options) -> Result<String>;

But I think this is better.

pub fn compile<R: io::Read, W: io::Write>(src: R, dst: W, options: &Options) -> Result<()>;

@kdy1 What do you think would be better and easier to interface with swc?

@kdy1
Copy link
Member Author

kdy1 commented Aug 11, 2020

@pickfire Does it compile scss to css?
String seems acceptable to me.

@pickfire
Copy link
Contributor

@pickfire Does it compile scss to css?

Yes, the output is css. That is basically what the crate is doing, I don't see it being useful to compile sass to sass. Haha

String seems acceptable to me.

Acceptable and even used by dart-sass. But not very flexible and performant, requires additional allocation for full String unlike write which can write partially to file directly wrapped in BufWriter. serde_json also provides these and I find it helpful.

@connorskees
Copy link

@pickfire In fact grass did originally have an API similar to the one you describe. It was necessary to move away from this however in order to support the post-processing done by Sass. Specifically, we must check if the entire output is ascii in order to emit @charset "UTF-8" (or a UTF-8 BOM in compressed mode) at the start of the string. Both libsass and dart-sass write first to an internal buffer for this reason.

Profiling has shown that this has very little impact on the actual performance, presumably because it is a one time cost compared to selector extension or style parsing which are performed hundreds to thousands of times for libraries like Bootstrap. I think the API you have described may be a form of premature optimization.

@pickfire
Copy link
Contributor

@kdy1 So you are planning to write your own css parser instead of using something like cssparser from servo?

@kdy1
Copy link
Member Author

kdy1 commented Aug 11, 2020

@kdy1 So you are planning to write your own css parser instead of using something like cssparser from servo?

I'm planning to write a new one. CSS does not have tricky syntax, so it will not be hard

@kdy1 kdy1 force-pushed the css branch 3 times, most recently from 1414084 to 16faff9 Compare August 17, 2020 06:30
@kdy1
Copy link
Member Author

kdy1 commented Aug 22, 2020

@connorskees Does grass support getting original source line?

@pickfire
Copy link
Contributor

@kdy1 Grass does that internally but not during the css output, IIRC there is no sourcemap support yet.

@@ -10,7 +10,23 @@ pub struct Text {
#[ast_node]
pub struct Stylesheet {
pub span: Span,
Copy link
Contributor

Choose a reason for hiding this comment

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

@kdy1 Does swc usually keep span for all the struct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, many operations related to comment or a source map depend on the span.

@CLAassistant
Copy link

CLAassistant commented Sep 27, 2020

CLA assistant check
All committers have signed the CLA.

@0rvar
Copy link

0rvar commented Mar 9, 2021

How far away is this PR? Anything that can be done to help it along?

@kdy1
Copy link
Member Author

kdy1 commented Mar 10, 2021

@0rvar Most hard part is determining ast.
All other works except it is very easy to me.
But I don't have time to do it.

@kdy1
Copy link
Member Author

kdy1 commented Aug 14, 2021

Closing in favor of #2074

@kdy1 kdy1 closed this Aug 14, 2021
@kdy1 kdy1 deleted the css branch October 5, 2022 12:44
@swc-project swc-project locked as resolved and limited conversation to collaborators Nov 5, 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.

css support
5 participants