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

Compile Twitter Bootstrap 4 #4

Closed
20 tasks done
connorskees opened this issue Apr 21, 2020 · 18 comments
Closed
20 tasks done

Compile Twitter Bootstrap 4 #4

connorskees opened this issue Apr 21, 2020 · 18 comments

Comments

@connorskees
Copy link
Owner

connorskees commented Apr 21, 2020

A 1.0 release of grass should be able to compile bootstrap identically to dart-sass. The features currently blocking Bootstrap from even compiling are

  • @extend
  • @keyframes
  • @supports
  • map as value in map
  • strange issue involving color functions and precision (this turned out to be due to a lack of rounding in the output of builtin functions red, green, and blue)
  • short circuiting of and operator
  • short circuiting of or operator
  • control flow inside style inside control flow
  • better handling of @import
  • self referential default function arguments

The places where grass and dart-sass differ in output are

  • attribute quoting is incorrect (e.g. output should be ["foo"] => [foo], but we retain quotes). dart-sass has complex rules for determining whether quotes should be emitted or not
  • newlines after multiline comments are retained for some reason in certain conditions. The newlines should be stripped.
  • extended selectors should be wrapped in Rc<RefCell<Selector>>
  • all sorts of media query features
    • variables are evaluated when inside parens
    • validation
    • we emit too many linebreaks inside @media
    • complex rules for emitting newlines around @media
  • line breaks after comments in selectors are not retained
  • division with / should only occur in special cases
@MartinKavik
Copy link

Is possible that these improvements allow to compile also Bulma .scss files?

@pickfire
Copy link
Contributor

pickfire commented May 4, 2020

@MartinKavik @connorskees Is still working on @extend which bulma uses.

From what I see from the source code, scss is supported too.

@connorskees
Copy link
Owner Author

@MartinKavik @pickfire yes that's correct, Bulma is blocked largely on @extend. Bulma also appears to use @keyframes in 1 place, which is trivially possible to implement right now, but would need to be completely rewritten in the course of implementing @extend, so I am holding off on implementing it at the moment.

A possibility could be silently eating @extend (maybe print a warning that it was eaten) rather than panicking with todo!(). I'll test doing this tomorrow and and release it to crates.io if it works well.

At the moment, only scss is supported. When the indented syntax, sass, is implemented, it will only ever be a second class citizen.

@MartinKavik
Copy link

Thanks for answer. I will be updating Seed template for new apps in next weeks so I'm just exploring options - there can be Bulma, Bootstrap or nothing and I can change it in the future - don't hurry, great job!

@pickfire
Copy link
Contributor

pickfire commented May 7, 2020

@connorskees Thanks for the work on this. By the way, are you planning to add a benchmark with the official dart implementation later on?

@connorskees

This comment has been minimized.

@connorskees

This comment has been minimized.

@connorskees connorskees pinned this issue Jun 24, 2020
@connorskees
Copy link
Owner Author

@MartinKavik grass is now able to compile the most recent verison of bulma-scss.

I'm still trying to track down an @extend bug that results in a combinatorial explosion when extending certain pseudoselectors, but other than that and some extra newlines the output is the exact same as dart-sass.

Performance has regressed slightly as a result of implementing @extend, but grass still appears to beat sassc and dart-sass in most benchmarks. Due to the bug I've mentioned, sassc beats grass in compiling bulma, but I would expect that disparity to disappear once I resolve it.

I'll be releasing a new bugfix version to crates.io once I can resolve the @extend issue.

@connorskees
Copy link
Owner Author

It is now possible to compile bulma-scss 100% accurately!*

* ignoring some newlines

You can play around with it by running,

git clone https://github.com/j1mc/bulma-scss
cargo install grass --force
grass bulma-scss/bulma.scss>out.css

There are currently 1 bug and 1 feature blocking bootstrap. The bug being something related to colors and number precision (I think grass is too precise!) and the feature being more robust parsing of @keyframes (specifically percent selectors). After these two are resolved, I have confirmed that grass will be able to compile the most recent version of bootstrap.

@MartinKavik
Copy link

It is now possible to compile bulma-scss 100% accurately!

Awesome news! I'm just writing the third and the biggest Seed tutorial and I want to use bulma and grass. It will be published on seed-rs.org.

@connorskees
Copy link
Owner Author

connorskees commented Jul 4, 2020

I've just released grass v0.9.3, which includes a 3x improvement for bulma compile time. With these optimizations, grass is now roughly 25% faster than libsass+sassc

Benchmark results
hyperfine "grass-new bulma-scss\bulma.scss" --warmup 5 
Benchmark #1: grass bulma-scss\bulma.scss
  Time (mean ± σ):     186.7 ms ±   5.6 ms    [User: 6.4 ms, System: 5.0 ms]
  Range (min … max):   176.0 ms … 200.6 ms    15 runs


hyperfine "sassc bulma-scss\bulma.scss" --warmup 5 
Benchmark #1: sassc bulma-scss\bulma.scss
  Time (mean ± σ):     244.1 ms ±  16.2 ms    [User: 5.7 ms, System: 5.3 ms]
  Range (min … max):   218.1 ms … 263.3 ms    10 runs

(to anyone subscribed to this issue, I'll try to avoid pinging this thread again until bootstrap is able to be compiled)

@connorskees
Copy link
Owner Author

Sorry to alert everyone so soon, but I was not expecting to finish this tonight!

grass v.0.9.4 is able to compile the most recent version of twitter bootstrap.

To test it out,

git clone https://github.com/twbs/bootstrap --depth 1
cargo install grass --force
grass bootstrap/scss/bootstrap.scss>out.css

I'll be leaving this issue open until I can resolve the extra newlines and special cased division.

@pickfire
Copy link
Contributor

pickfire commented Jul 5, 2020

@MartinKavik Off-topic, I am wondering if seed will ever look into not using virtual DOM?

@MidasLamb
Copy link
Contributor

MidasLamb commented Aug 12, 2020

For me, bootstrap 4.5.2 doesn't compile with version 0.10.0, nor with 0.9.4:

Error: Expected "important".
  ╷
9 │ $gray-100: #f8f9fa !default;
  │                    ^^^^^^^^
  ╵
././scss/_variables.scss:9:20

@connorskees
Copy link
Owner Author

@MidasLamb You're right! Thank you for bringing this up. The issue is almost certainly related to a bug recently introduced in one of our dependencies. The error was introduced in a bugfix release so cargo is pulling in the broken version. This also broke CI. See 0c7e201 for a recent fix for this. I will release 0.10.1 today and have the Cargo.lock included on crates.io in order to avoid this occurring in the future.

@MidasLamb
Copy link
Contributor

@connorskees , great, thanks for the quick response!

@connorskees
Copy link
Owner Author

@MidasLamb for now unfortunately it seems that publishing 0.10.1 is blocked on our dependency publishing the patch to crates.io. I've opened an issue there so hopefully we won't have to wait too long.

If you'd like to use grass now, you can install it with cargo install --git https://github.com/connorskees/grass, which I have confirmed to work with the version of Bootstrap on my machine.

@connorskees
Copy link
Owner Author

The next release of grass will have byte-for-byte accurate output for bootstrap v5.0.2, verified by CI.

Additionally, I have run a script to perform this check for the last 2,500 commits of bootstrap and have resolved all the differences (largely strange handling of newlines). This commit range includes all commits of bootstrap 4 and 5.

I am considering the work necessary to compile bootstrap done. Remaining features/bugs are tracked in #19.

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

No branches or pull requests

4 participants