StrataSource / Engine

Issue tracker for Strata Source
47 stars 2 forks source link

Bug: Panorama CSS parser errors on missing terminating semicolon #253

Closed leops closed 2 years ago

leops commented 2 years ago

Describe the bug

Per the CSS language specs, semicolon tokens are only required as separator between declarations. This means the following rule is valid CSS:

.className {
    height: 32px;
    width: 96px /* <- no semicolon here */
}

However the CSS parser used by Panorama requires that all declarations are terminated by a semicolon. This is problematic when using a CSS preprocessor (such as PostCSS in Finnby) that sometimes omit the terminating semicolon: the generated code is spec-compliant CSS but does not load in Panorama

To Reproduce

  1. Add the aforementioned rule in a Panorama stylesheet
  2. Panorama emits an error when the file gets loaded

Expected Behavior

The rule should be parsed and applied correctly

Operating System

No response

lvaness commented 2 years ago

The panorama CSS parser is by no means a conformant implementation of CSS and is also not supposed to be

lvaness commented 2 years ago

I'm going to mark this as needs more info as I don't see any concrete usage scenario why this absolutely needs to be supported. PostCSS supports - through the default configuration of the stylefmt plugin - adding semicolons to the end of each block

leops commented 2 years ago

Yeah I agree this is low priority, I just opened this here for bookkeeping. As you mentioned it's possible to get around the issue in PostCSS with plugins (although I'm just using a simpler ad-hoc plugin to force trailing semicolon insertion on all blocks instead of shipping a whole styling library to Finnby), and I already have a regexp-based workaround (I don't like it, but it works) in place for inline styles in React DOM.

But arguably the whole point of Panorama is to make developing interfaces for Source more accessible by using well-known technologies, and removing inconsistencies with the web platform is useful in that regard as it makes it easier for developers to leverage existing knowledge without hitting minor inconveniences all the time, as well as using existing tooling without having to tweak the configuration or introducing hacks.

JJL772 commented 2 years ago

Hasn't this been implemented?

leops commented 2 years ago

Yup, it's in the 0.6.0 release