Hoist statement at-rules to the front of the aggregated stylesheet

CHANGES.md

@@ -2,6 +2,7 @@
 
 ## 0.62.0
 
+- [FIX] Hoist `@import` rules, then `@namespace` rules, to the front of `styled-ppx.generate`'s output, regardless of which module or library emitted them, since a browser only honors these rules when they precede every other rule, dependency order routinely places one after plain style rules, and CSS Cascade 5 requires every `@import` to precede every `@namespace`. Statement-form `@layer a, b;` is left where it falls: CSS allows it anywhere in a stylesheet, and hoisting it would change layer order (layer order is first occurrence). Classify by the rendered rule's text (`@import`/`@namespace` prefix, `;` suffix) instead of by the presence of a `{`, since an `@import` URL can itself contain `{`. Drop `@charset` instead, with a warning naming the input file: the generated file always opens with its own leading comment, so `@charset` can never be the literal first bytes of the stylesheet, and the output is UTF-8 regardless (#581) (@davesnx)
 - [FIX] Parse the Selectors Level 4 `An+B of S` form of `:nth-child()`/`:nth-last-child()` (`:nth-child(2n+1 of .x)`), and functional pseudo-elements such as `::part(foo)` and `::slotted(.bar)`. The lexer only special-cased `nth-*` names for An+B payloads and gave every other identifier followed by `(` a generic function token that `::`-parsing never handled, so both forms hit a raw parse error; the renderer already had support for the "of S" AST shape, just not a parser path to it (@davesnx)
 - [FIX] Accept CSS Nesting's relative-selector shorthand in a nested rule's prelude: `.parent { > .child {} }`, `+ .sib {}` and `~ .sib {}` resolve exactly like `& > .child`. The root of `[%css]`, `[%styled.global]` and a raw stylesheet still rejects a leading combinator, and `[%styled.global]` now also rejects one inside an at-rule block with no enclosing style rule (`@media print { > .a {} }`), the same way it rejects a parentless `&`, instead of shipping unresolved `&` (@davesnx)
 - [FEATURE] Register the css-fonts-5 metric-override descriptors `ascent-override`, `descent-override`, `line-gap-override` (each `normal | <percentage>`), and `size-adjust` (`<percentage>`), so a valid `@font-face` block declaring them compiles instead of failing with an unknown-descriptor error (#580) (@davesnx)
@@ -43,7 +44,7 @@
 - [FIX] Fix compiler crashes when parsing legacy `-webkit-gradient(...)` and `-webkit-mask-box-image` values whose internal registry references omitted the leading dash. A registry-closure check now validates grammar lookups with the same key derivation as generated parsers (@davesnx)
 - [FIX] Parse `:nth-*()` An+B syntax correctly, including uppercase forms, `n-3`, and `2n- 3`. Invalid units, fractional or oversized coefficients, and malformed payloads now produce located errors instead of truncation or `Failure("int_of_string")` crashes (@davesnx)
 - [FIX] Report invalid UTF-8 in CSS payloads at the offending byte instead of crashing with `Sedlexing.MalFormed` (@davesnx)
-- [FIX] Hoist statement at-rules such as `@import`, `@namespace`, and statement-form `@layer` before style rules while preserving their relative order (@davesnx)
+- [FIX] Hoist `@import` then `@namespace` statement at-rules before style rules while preserving each kind's relative order; statement-form `@layer` is left in place instead, since CSS allows it anywhere and hoisting it would change layer order (@davesnx)
 - [FIX] Point interpolation, bare-pseudo, and `@media` prelude errors at the offending source token, with compiler excerpts and carets (@davesnx)
 - [FIX] Generate `makeProps` and a public `make` wrapper for native components, fixing `Unbound value X.makeProps` with the new server-reason-react JSX transform (@davesnx)
 - [FIX] Support `[%css]` inside `include struct ... end` (@davesnx)

documents/css-extraction.md

@@ -397,17 +397,47 @@ deduped through `Set.Make(String)`, which sorted by hash-prefixed rule
 text and silently destroyed declaration order (regression test:
 `packages/generate/test/source-order.t`).
 
-The deduplicated list is then written to the output channel. Inter-rule
-newlines are dropped when every contributing input file declared
-`env=production` in its `[@@@css.config ...]` (see the wire protocol
-section above); there is no CLI flag for this.
+Before writing, `@import` rules are hoisted to the front of the
+deduplicated list, then `@namespace` rules right after them, each block
+keeping its own relative order, regardless of which library or module
+emitted them: a browser only honors these rules when they precede every
+other rule, Order above routinely places the module that emits one after
+modules with plain style rules, and CSS Cascade 5 additionally requires
+every `@import` to precede every `@namespace`. Classification is a string
+test on the rendered rule — starts with `@import`/`@namespace` and ends
+with `;` — rather than a search for `{`, since an `@import` URL can itself
+contain `{` (`@import url("a{b.css");` is a valid statement, not a block
+rule). Statement-form `@layer a, b;` is deliberately NOT hoisted: CSS
+allows it anywhere in a stylesheet, and layer order is first-occurrence
+order, so relocating a `@layer` statement would silently reorder a
+library's cascade layers instead of just moving text. It stays wherever
+dedup/ordering placed it, which under `--layers` (below) means inside its
+own library's `@layer { ... }` block, where it declares sub-layers scoped
+to that library's rules — a normal and supported use of statement-form
+`@layer`.
+`@charset` is dropped instead of hoisted: the generated file always opens
+with its own leading comment, so `@charset` can never be the literal
+first bytes of the stylesheet, and the file is written as UTF-8 regardless
+of what a module declares; dropping it is reported as a warning naming
+the input file (`packages/generate/test/statement-at-rules.t`).
+
+The deduplicated (and hoisted) list is then written to the output
+channel. Inter-rule newlines are dropped when every contributing input
+file declared `env=production` in its `[@@@css.config ...]` (see the wire
+protocol section above); there is no CLI flag for this.
 
 ### Cascade layers (opt-in)
 
 `--layers` (default off, and rejected together with `--order source`)
 wraps the deduplicated rule list from above into named CSS cascade
-layers, one per library, instead of one flat list. `@property` and
-`@keyframes` rules are pulled out ahead of every layer, since they are
+layers, one per library, instead of one flat list. Hoisted `@import` and
+`@namespace` rules stay ahead of everything below, including the
+registrations, the aggregator's own `@layer <lib1>, <lib2>, ...;`
+statement, and every wrapped block, for the same reason they are hoisted
+in the unlayered case. A statement-form `@layer a, b;` is not part of
+this hoisted set, so it falls into its library's own `@layer { ... }`
+block alongside that library's other rules. `@property` and
+`@keyframes` rules are pulled out ahead of every layer next, since they are
 global registrations: a `@property` inside a layer would make the
 registration itself depend on layer order, and a `@keyframes` name is
 looked up by layer order too, so leaving both unlayered avoids surprises.

packages/generate/generate.ml

@@ -538,6 +538,37 @@ let is_global_registration rule =
   String.starts_with ~prefix:"@property" trimmed
   || String.starts_with ~prefix:"@keyframes" trimmed
 
+(** A rendered rule is a hoisted [\@import]/[\@namespace] statement when, after
+    trimming, it starts with that keyword and ends with [';']. CSS only honors
+    these two kinds when they precede every other rule (and Cascade 5
+    additionally requires every [\@import] to precede every [\@namespace]), so
+    the aggregator hoists them to the front of the output — [\@import] block
+    first, then [\@namespace] — regardless of which module emitted a rule or
+    where {!order_by_dependency} placed that module; relative order within each
+    kind is left alone. Statement-form [\@layer a, b;] is deliberately NOT
+    hoisted here: CSS permits it anywhere in a stylesheet, and layer order is
+    first-occurrence order, so moving a [\@layer] statement would silently
+    reorder a library's cascade layers instead of just relocating text. Textual
+    prefix/suffix matching (not a search for ['{']) is required because an
+    [\@import] URL can itself contain ['{'] (e.g. [\@import url("a{b.css");]),
+    which a ['{']-search would misclassify as a block rule. *)
+let is_import_statement rule =
+  let trimmed = String.trim rule in
+  String.starts_with ~prefix:"@import" trimmed
+  && String.ends_with ~suffix:";" trimmed
+
+let is_namespace_statement rule =
+  let trimmed = String.trim rule in
+  String.starts_with ~prefix:"@namespace" trimmed
+  && String.ends_with ~suffix:";" trimmed
+
+(** [\@charset] is only ever honored as the literal first bytes of a stylesheet,
+    but the generated file always opens with a comment identifying it and is
+    written as UTF-8 regardless of what a module declares, so a collected
+    [\@charset] can never be honored and is dropped (with a warning) instead of
+    silently misplaced. *)
+let is_charset rule = String.starts_with ~prefix:"@charset" (String.trim rule)
+
 (** Collect, index, resolve, dedup, output. *)
 let run ~output_file ~order ~layers input_files =
   Logger.info "output file: %s"
@@ -608,7 +639,13 @@ let run ~output_file ~order ~layers input_files =
             Css_extraction.resolve_sentinels ~lookup:(Index.lookup idx)
               ~on_unresolved:on_error ~on_malformed rule
           in
-          resolved_rules := (resolved, input_layer) :: !resolved_rules)
+          if is_charset resolved then
+            Logger.warning
+              "%s: dropping @charset: the output already starts with a leading \
+               comment, so @charset could never be the first bytes of the \
+               stylesheet; output is UTF-8 regardless."
+              input.filename
+          else resolved_rules := (resolved, input_layer) :: !resolved_rules)
         input.rules)
     inputs;
 
@@ -646,6 +683,19 @@ let run ~output_file ~order ~layers input_files =
       end)
   in
 
+  (* [@import] before [@namespace] (Cascade 5), each preserving its own
+     relative order; everything else, [@layer] statements included, stays in
+     [other_rules] untouched. *)
+  let import_rules, rest =
+    List.partition
+      (fun (rule, _layer) -> is_import_statement rule)
+      ordered_rules
+  in
+  let namespace_rules, other_rules =
+    List.partition (fun (rule, _layer) -> is_namespace_statement rule) rest
+  in
+  let statement_rules = import_rules @ namespace_rules in
+
   let minify = production_mode inputs in
   Logger.info "environment: %s"
     (if minify then "production (from [@@@css.config])" else "development");
@@ -654,24 +704,20 @@ let run ~output_file ~order ~layers input_files =
     let buffer = Buffer.create 1024 in
     Buffer.add_string buffer
       "/* This file is generated by styled-ppx, do not edit manually */\n";
+    let emit (rule, _layer) =
+      Buffer.add_string buffer rule;
+      Buffer.add_string buffer separator
+    in
+    List.iter emit statement_rules;
     (match layer_names with
-    | None ->
-      List.iter
-        (fun (rule, _layer) ->
-          Buffer.add_string buffer rule;
-          Buffer.add_string buffer separator)
-        ordered_rules
+    | None -> List.iter emit other_rules
     | Some layer_names ->
       let registrations, library_rules =
         List.partition
           (fun (rule, _layer) -> is_global_registration rule)
-          ordered_rules
+          other_rules
       in
-      List.iter
-        (fun (rule, _layer) ->
-          Buffer.add_string buffer rule;
-          Buffer.add_string buffer separator)
-        registrations;
+      List.iter emit registrations;
       (* A repeated name in the statement doesn't declare a second layer, it
          only re-mentions the same slot, so list each name once (first
          occurrence): two colliding keys still get their own [@layer name {

packages/generate/test/statement-at-rules.t/run.t

@@ -0,0 +1,108 @@
+Statement at-rules (`@import`, `@namespace`) must precede every other rule
+for a browser to honor them, and CSS Cascade 5 additionally requires every
+`@import` to precede every `@namespace`. The aggregator hoists these two
+kinds to the front of the output, `@import` block then `@namespace` block,
+each keeping its own relative order, regardless of which module emitted them
+or in what order: here `z_statements.ml` is ordered *after* `a_styles.ml`
+(plain alphabetical order, no dependency edge between them), yet its
+statements still come first.
+
+`@charset` is dropped instead: the generated file always opens with its own
+comment, so `@charset` could never be honored as the first bytes of the
+stylesheet, and the output is UTF-8 regardless of what a module declares.
+Dropping it is reported as a warning naming the input file.
+
+Statement-form `@layer a, b;` is NOT hoisted: CSS allows it anywhere in a
+stylesheet, and moving it would change layer order (layer order is first
+occurrence, so relocating a `@layer` statement can silently reorder every
+library's cascade layer). It stays wherever dedup/ordering placed it, so
+here it is emitted after `a_styles.ml`'s rule, not before it.
+
+  $ cat > a_styles.ml <<EOF
+  > [@@@css ".a{color:red;}"]
+  > EOF
+
+  $ cat > z_statements.ml <<EOF
+  > [@@@css "@charset \"utf-8\";"]
+  > [@@@css "@import url(\"reset.css\");"]
+  > [@@@css "@namespace svg url(\"http://www.w3.org/2000/svg\");"]
+  > [@@@css "@layer utilities, base;"]
+  > EOF
+
+  $ styled-ppx.generate a_styles.ml z_statements.ml
+  styled-ppx: z_statements.ml: dropping @charset: the output already starts with a leading comment, so @charset could never be the first bytes of the stylesheet; output is UTF-8 regardless.
+  /* This file is generated by styled-ppx, do not edit manually */
+  @import url("reset.css");
+  @namespace svg url("http://www.w3.org/2000/svg");
+  .a{color:red;}
+  @layer utilities, base;
+
+`--layers` wraps each library's rules in a named `@layer`. `@import` and
+`@namespace` must precede both the aggregator's own `@layer <libraries>;`
+statement and every wrapped block. The un-hoisted `@layer utilities, base;`
+statement falls inside its library's wrapped block, alongside the rules it
+shares a library with; that is fine, since a statement-form `@layer` can
+declare sub-layers from anywhere.
+
+  $ styled-ppx.generate --layers a_styles.ml z_statements.ml
+  styled-ppx: z_statements.ml: dropping @charset: the output already starts with a leading comment, so @charset could never be the first bytes of the stylesheet; output is UTF-8 regardless.
+  /* This file is generated by styled-ppx, do not edit manually */
+  @import url("reset.css");
+  @namespace svg url("http://www.w3.org/2000/svg");
+  @layer _;
+  @layer _ {
+  .a{color:red;}
+  @layer utilities, base;
+  }
+
+`@import` before `@namespace` is required even when a module writes them in
+the opposite order: here `b_reversed.ml` declares `@namespace` first, but
+the aggregator still emits `@import` first.
+
+  $ cat > b_reversed.ml <<EOF
+  > [@@@css "@namespace svg url(\"urn:svg\");"]
+  > [@@@css "@import url(\"reset.css\");"]
+  > EOF
+
+  $ styled-ppx.generate b_reversed.ml
+  /* This file is generated by styled-ppx, do not edit manually */
+  @import url("reset.css");
+  @namespace svg url("urn:svg");
+
+A statement-form `@layer x, y;` that follows a block-form `@layer x { ... }`
+stays after it: hoisting it ahead (as the old `{`-search classifier did,
+since neither rule string contains a lone `{` outside the block) would move
+`x`'s first occurrence, and with it the layer's cascade position.
+
+  $ cat > c_layer_order.ml <<EOF
+  > [@@@css "@layer base {.a{color:red;}}"]
+  > [@@@css "@layer theme, base;"]
+  > EOF
+
+  $ styled-ppx.generate c_layer_order.ml
+  /* This file is generated by styled-ppx, do not edit manually */
+  @layer base {.a{color:red;}}
+  @layer theme, base;
+
+An `@import` URL that itself contains `{` must still be classified by its
+`@import ... ;` text, not by scanning for a literal `{`: the old classifier
+took any `{` as proof of a block rule and, under `--layers`, would emit this
+`@import` inside a `@layer { ... }` block, where it is invalid.
+
+  $ cat > d_import_brace.ml <<EOF
+  > [@@@css "@import url(\"a{b.css\");"]
+  > [@@@css ".d{color:green;}"]
+  > EOF
+
+  $ styled-ppx.generate d_import_brace.ml
+  /* This file is generated by styled-ppx, do not edit manually */
+  @import url("a{b.css");
+  .d{color:green;}
+
+  $ styled-ppx.generate --layers d_import_brace.ml
+  /* This file is generated by styled-ppx, do not edit manually */
+  @import url("a{b.css");
+  @layer _;
+  @layer _ {
+  .d{color:green;}
+  }

packages/ppx/test/css-support/at-rule-statement-forms.t/run.t

@@ -5,9 +5,9 @@ already do.
   $ refmt --parse re --print ml input.re > output.ml
   $ ../../standalone.exe --impl output.ml -o output.ml
   $ styled-ppx.generate output.ml > styles.css
+  styled-ppx: output.ml: dropping @charset: the output already starts with a leading comment, so @charset could never be the first bytes of the stylesheet; output is UTF-8 regardless.
   $ cat styles.css
   /* This file is generated by styled-ppx, do not edit manually */
-  @charset "utf-8";
   @import url("reset.css");
   @namespace svg url("http://www.w3.org/2000/svg");
   @layer utilities;