grass icon indicating copy to clipboard operation
grass copied to clipboard

Known Outstanding Issues Megathread

Open connorskees opened this issue 4 years ago • 14 comments

This issue serves as a way to keep track of all known compilation issues

Large features

  • [x] @use and the module system (MVP)
  • [x] @forward #67
  • [x] indented syntax #67
  • [x] compressed output
  • [x] plain-CSS imports #67

Smaller features

These ideally come with minimal reproductions, though the reason why they're failing may be incorrect as they haven't been fully looked into

  • [x] comma separated imports
    • @import "hey1.css", "cookie.css", url("hey2.css"), "fudge.css";
  • [x] parens surrounding single word in media query
    • @media (color) {a {color: red;}}
  • [x] min as arg to min/max
    • min(1, min(2))
  • [x] special function as arg to min/max
    • min(1, env(--foo))
  • [x] inequality sign inside interpolation in media query
    • @media (#{"100px < width < 500px"}) {a {interpolation: in-parens}}
  • [ ] invalid simple selector (should error, but panics)
    • simple-selectors(">")
  • [x] vendor prefixed known at rules (one of @supports, @keyframes)
    • @-webkit-keyframes {}
  • [x] styles can render at the toplevel, but they should not be able to #67
    • color: red; should error
  • [x] zero divided by zero panics, but should return NaN
    • (0 / 0)
  • [x] one divided by zero panics, but should return Infinity (this has been updated to return an error instead of crashing) #67
    • (1 / 0)
  • [x] division should only occur when either of the operands is calculated or if they operation is in parens
    • 1 / 2 should be emitted as 1/2, but it currently gives 0.5
  • [x] CSS variables should include everything, notably comments #67
    • --foo: //;
  • [x] numbers should use fuzzy matching -- that is, they should be equal if they are the same to 10 decimal points #67
    • .9999999999999999999999999999999 == .99999999999999999999999999999998
  • [x] we should allow a BOM at the start of files
  • [x] all instances of @import should be moved to the top of the file
  • [x] rgb(a) functions should allow / #67
    • rgba(1, 2, 3 / 4)
  • [x] we dont retain empty parens inside url(...) #67
    • url((((())))) (this should actually produce an error, but it just emits url()
  • [x] @import should include strings verbatim #67
    • @import "hux\ bux.css"; this should include the \, but it does not because we use normal string parsing
  • [x] subtraction with no space between operands and rhs is interpolated
    • 10-#{10} should give 10 -10 but it gives 10-10
  • [x] non-comparable inverse units #67
    • (1px / 1em)
  • [x] !optional in @extend
  • [x] deny content block being passed to @mixin without @content #67
  • [ ] @extend should not be possible between media query boundaries
  • [x] @media query merging #67
  • [x] inspect(...) does not properly preserve parens in some cases
  • [x] @charset should only accept a string
    • @charset 1+1; should error
  • [x] function names containing interpolation should always be evaluated as unknown functions #67
    • color: qu#{o}te(red) current returns "red", but should give quote(red)
  • [x] unknown at rule with query but no curly braces
    • @foo "bar"; (we do parse this correctly, but we emit it with two semicolons)
  • [x] unicode ranges
    • e.g. U+A2??
  • [x] allow varargs to contain named arguments and support builtin fn keywords #67
  • [x] feature complete parsing of @supports #67
  • [x] support angle units other than deg to builtin fns, math.cos(1grad) #67

connorskees avatar Jul 07 '20 01:07 connorskees

@connorskees I am interested to help out on compressed output, are you working on that? If possible, maybe you can write out some mentor instructions?

pickfire avatar Jul 07 '20 07:07 pickfire

It looks like it's just missing compressed output to be a good replacement to sass-rs? Would you consider this library ready @connorskees ?

Keats avatar Dec 06 '20 16:12 Keats

@Keats I think it is close to being ready! There's basically the same underlying issue preventing a number of features that I would like to resolve before I am willing to call the library ready. I'd like to get this library over the finish line in the first quarter of 2021, assuming I have enough free weekends.

I think, though, if given the explicit warning that the library is experimental and may not work for all features of the language, it may be fine to rely on grass in an opt-in fashion. The known features remaining are admittedly quite niche (as far as I can tell) -- specifically I am concerned about @media query merging, @extend inside @media, complex uses of @at-root, invalid syntax inside plain-CSS variables, and certain arguments to min/max.

There shouldn't be any more breaking API changes for quite some time.

I think if you check back in 2-3 weeks, it should be in a good enough state to use experimentally. And then in another 2-3 months it should hopefully be in a good enough state to use seriously.

connorskees avatar Jan 03 '21 06:01 connorskees

Nice! I'll make it an opt-in feature of the next version of Zola. The listed features are pretty niche I think so it shouldn't cause issues for most users I think and I can finally drop compass soon-ish.

Keats avatar Jan 03 '21 07:01 Keats

cc @kdy1 who may be interested for https://github.com/swc-project/swc/pull/950

pickfire avatar Jan 03 '21 15:01 pickfire

@connorskees is this project still active?

Keats avatar May 06 '21 11:05 Keats

hey @Keats, yes this project has recently come back to life. I had to take a hiatus at the beginning of the year, but I am hoping to wrap this project up. I have made a number of improvements that should resolve many of the concerns I had earlier.

This project should be fine to use in a production setting. I have verified that grass' output is byte-for-byte the same with dart-sass for the last 2,500 commits to bootstrap, which includes all of bootstrap 4 and 5.

connorskees avatar Jul 25 '21 08:07 connorskees

@connorskees I think compressed output is not as good as the original one yet. There are quite some optimizations that we could do. Also, I think we can do better maybe. So not sure if you want to mark that as checked.

pickfire avatar Jul 25 '21 09:07 pickfire

@pickfire We actually have a pretty good amount of compressed output-parity. I've just pushed up a commit adding support for compressed number and list values. I think the number of bytes we could further remove is quite small and will not affect most users.

For reference, the features of dart-sass's compressed output are enumerated here.

connorskees avatar Jul 25 '21 13:07 connorskees

Hey thanks for this awesome crate!

I'm curious where the indented syntax is on your plan, time wise?

In my projects I mostly use Bulma instead of Bootstrap and they only provide the indented SASS syntax instead of SCSS.

Would love to jump off the sass-rs crate and use grass in the future, the indented syntax support currently being my only blocker.

dnaka91 avatar Nov 25 '21 02:11 dnaka91

Hi @dnaka91, I've added support for the indented syntax in #67, which should be released to crates.io with a minor version bump at some point this week.

I have verified the output for bulma against dart-sass, and there are no differences.

connorskees avatar Dec 26 '22 20:12 connorskees

Thank you, @connorskees. I finally gave it a try. Works like a charm, and now I could finally move away from using sass-rs.

Had some initial issues because I was mixing SASS with plain CSS, which apparently sass-rs can work with. But wasn't a big deal and I could work around that pretty quickly.

dnaka91 avatar Apr 11 '23 03:04 dnaka91

I'm trying to use Pico CSS v2.0.0-alpha1 with Zola, and I'm getting the error that map.deep-merge is an undefined function. Reading the documentation, it seems it's supported by Dart-Sass, so it should be in the scope of grass?

Here's the error I get by unpacking pico-2/scss under sass while running zola serve:

Error: Error: Undefined function.
   ╷
40 │ $breakpoints: map.deep-merge(
   │               ^^^^^^^^^^^^^
   ╵
.../sass/_settings.scss:40:15

I tried removing the prefix, but that didn't go as expected:

Error: Error: (sm: (breakpoint: 576px, viewport: 510px, root-font-size: 106.25%), md: (breakpoint: 768px, viewport: 700px, root-font-size: 112.5%), lg: (breakpoint: 1024px, viewport: 950px, root-font-size: 118.75%), xl: (breakpoint: 1280px, viewport: 1200px, root-font-size: 125%), xxl: (breakpoint: 1536px, viewport: 1450px, root-font-size: 131.25%)) isn't a valid CSS value.
   ╷
40 │ $breakpoints: deep-merge(
   │               ^^^^^^^^^^^^^
   ╵
.../sass/_settings.scss:40:15

What should I do? I'm not sure where the fault lies exactly—I'm a back-end dev ;)

ISSOtm avatar Aug 21 '23 22:08 ISSOtm

@ISSOtm grass does support this function, but in a version that zola hasn't yet picked up (added in 0.12.4, zola uses 0.12.3). One solution to this until zola does its next release would be to build zola from source after deleting the Cargo.lock file.

I manually ran the latest version of grass against the v2 branch of the pico library and it seems to compile fine.

connorskees avatar Aug 22 '23 16:08 connorskees