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

Towards 1.0 #77

Open
1 of 30 tasks
connorskees opened this issue May 17, 2023 · 6 comments
Open
1 of 30 tasks

Towards 1.0 #77

connorskees opened this issue May 17, 2023 · 6 comments

Comments

@connorskees
Copy link
Owner

connorskees commented May 17, 2023

grass is very close to completion and is approaching a 1.0 release. The API is stable and there are very few deviations from the reference implementation.

That said, there is still a bit of work to be done before I would want to call grass fully feature-complete and having full parity with the reference implementation. In its current state I would argue that grass is suitable for virtually all users of Sass, though there are still a few pieces missing.

This is a rough outline of my thoughts for what to work on next for this project. A large part of the remaining work is to catch up with the new features dart-sass has added in the last year or so.

The Module System

@use and @forward have a really surprising level of depth and complexity. Right now very few Sass projects use the module system, but as people move away from libsass, I expect that number to increase. There are a large number of failing spec tests related to this and I'm aware of at least 1 project that fails to compile due to its mixing of @import, @use, and @forward. This bullet is just to solve all of the remaining failing spec tests

  • feature complete module system

API

grass hasn't had a breaking change to its rust API in well over 2 years, and the API that exists currently is resilient to breaking changes. I consider this section largely feature complete, and unless new features are requested, grass will retain the same API it has currently in a 1.0 version.

I may want to add the ability to further configure grass::include! output options, but it should be possible to do so in a backwards-compatible way.

Features that rely on the internals of grass, such as Sass functions written in rust or AST traversal can be built through the internals crate grass_compiler.

Cosmetics

We differ from dart-sass in a number of ways that don't affect the semantics of the output, but do affect the byte-for-byte equality of the output. For the purposes of automated diffing, it would be nice to resolve these differences. Additionally, there are some places where I think grass can improve on things like error messages and error spans.

  • emit loud comments declared on the same line as a statement on the same line as that statement in expanded mode
    • a { /**/ } should emit a { /**/ }. This is a feature dart-sass has added in the last few months
  • remove invalid combinators
    • a > > b {} should not be emitted. This is again a very recent addition to dart-sass
  • do not emit plain CSS @import rules above loud comments that were declared above them
  • preserve certain whitespace for custom-property style rules with newlines (e.g. --foo: ...)
  • our logic for deciding when to split media queries is intentionally simplified, and sometimes splits too often. this makes us differ at times from dart-sass, though does not affect semantics
  • for certain error messages involving unitful NaN, we do not print NaN's unit
  • dart-sass solved their @media indentation bug that we had been emulating, so we should update our code as well

Devops

  • As close to 100% code coverage as possible
  • bulma in CI
  • other large sass frameworks (carbon, duomo, a11ycolor, etc.) in CI
  • multiple versions of bootstrap in CI
  • dart-sass differential fuzzing. grass is already quite regularly fuzzed for crashes, but differential fuzzing would let us find other kinds of bugs

Performance

I've implemented benchmarking in connorskees/sass-perf. grass is ~2x faster than dart-sass for all but 1 benchmark. I have a branch currently that improves SassMaps to make them proper hash maps with amortized constant time lookup and insertion. This doesn't fully fix performance issues on this benchmark, but does bring us a little bit closer. The remaining work here is

  • improve performance of variable lookups by not cloning values or making them cheaper to clone through Rc/Arc
  • improve performance of builtin function calls by not cloning values or making them cheaper to clone through Rc/Arc
  • more robust module/file caching system

An exploration into using Arc/Rc for all values improved performance on bootstrap by about 20-50%.

General compatibility

These are real issues that can affect the semantics of the outputted CSS or cause errors/crashes on valid input. In general, these are extremely rare issues that do not come up very often in practice, but they should be resolved before a final release

Misc

Future dart-sass compatibility

This section is intended to grow as dart-sass adds new features. As part of maintaining grass, new features and bug fixes must be periodically translated from dart-sass.

@jhpratt
Copy link

jhpratt commented Jul 26, 2023

  • Color spaces level 4 (not currently merged into dart-sass)

Adding media queries level 4 to this — @media (width < 1000px) {} is valid but currently rejected.

@connorskees
Copy link
Owner Author

@jhpratt What version of grass are you using? We do support media queries level 4.

You can try your example in a playground here. Perhaps you have an old version (prior to 0.12.0) or have a different reproducer?

@jhpratt
Copy link

jhpratt commented Jul 26, 2023

For some reason I didn't even consider that 🤦 I'm using it indirectly through Zola, and it turns out I'm using a slightly older version of that (which doesn't even rely on grass). Sure enough, the latest version there relies on 0.12. If zola were published on crates.io, it would have updated automatically along with my other crates, but unfortunately that's not the case.

Sorry for the mixup!

@clarfonthey
Copy link

Was poking around the repo but figured I'd ask here: would source mapping be a reasonable feature to add at some point? I haven't seen any mention of it so I'm not sure if this is undesired or just hasn't been considered yet.

@connorskees
Copy link
Owner Author

@clarfonthey Good question. Definitely something I've thought about before and have done some cursory research into. Generating source maps is a feature I would like to support but haven't had any time to start implementing. If you can share more about your use case I could potentially prioritize it.

So far no one has asked for source map support explicitly, and sass-rs (libsass bindings in rust) to my knowledge has never supported source maps so I assume it's a decently niche use case for this library, though I absolutely see the utility and think it would be a fun feature to work on.

@clarfonthey
Copy link

That's very fair. In my case, I was thinking even something simple like being able to tell what specific file a rule came from is helpful, since @import will mix that up.

It's definitely very difficult to make work, though.

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

3 participants