fix order of properties with all shorthand

fixes parcel-bundler#746

Author: devongovett

Cargo.lock

@@ -62,7 +62,6 @@ const-str = "0.3.1"
 pathdiff = "0.2.1"
 ahash = "0.8.7"
 paste = "1.0.12"
-indexmap = "2.2.6"
 # CLI deps
 atty = { version = "0.2", optional = true }
 clap = { version = "3.0.6", features = ["derive"], optional = true }

Cargo.toml

@@ -116,7 +116,7 @@ test('custom units', () => {
     }
   });
 
-  assert.equal(res.code.toString(), '.foo{font-size:calc(3*var(--step));--step:.25rem}');
+  assert.equal(res.code.toString(), '.foo{--step:.25rem;font-size:calc(3*var(--step))}');
 });
 
 test('design tokens', () => {
@@ -822,7 +822,7 @@ test('dashed idents', () => {
     }
   });
 
-  assert.equal(res.code.toString(), '.foo{color:var(--prefix-foo);--prefix-foo:#ff0}');
+  assert.equal(res.code.toString(), '.foo{--prefix-foo:#ff0;color:var(--prefix-foo)}');
 });
 
 test('custom idents', () => {

node/test/visitor.test.mjs

@@ -1,6 +1,7 @@
 //! CSS declarations.
 
 use std::borrow::Cow;
+use std::collections::HashMap;
 use std::ops::Range;
 
 use crate::context::{DeclarationContext, PropertyHandlerContext};
@@ -33,14 +34,13 @@ use crate::properties::{
   transition::TransitionHandler,
   ui::ColorSchemeHandler,
 };
-use crate::properties::{CSSWideKeyword, Property, PropertyId};
+use crate::properties::{Property, PropertyId};
 use crate::traits::{PropertyHandler, ToCss};
 use crate::values::ident::DashedIdent;
 use crate::values::string::CowArcStr;
 #[cfg(feature = "visitor")]
 use crate::visitor::Visit;
 use cssparser::*;
-use indexmap::IndexMap;
 
 /// A CSS declaration block.
 ///
@@ -516,10 +516,9 @@ pub(crate) struct DeclarationHandler<'i> {
   color_scheme: ColorSchemeHandler,
   fallback: FallbackHandler,
   prefix: PrefixHandler,
-  all: Option<CSSWideKeyword>,
   direction: Option<Direction>,
   unicode_bidi: Option<UnicodeBidi>,
-  custom_properties: IndexMap<DashedIdent<'i>, CustomProperty<'i>>,
+  custom_properties: HashMap<DashedIdent<'i>, usize>,
   decls: DeclarationList<'i>,
 }
 
@@ -571,13 +570,18 @@ impl<'i> DeclarationHandler<'i> {
       }
 
       if let CustomPropertyName::Custom(name) = &custom.name {
-        if let Some(prev) = self.custom_properties.get_mut(name) {
-          if prev.value == custom.value {
+        if let Some(index) = self.custom_properties.get(name) {
+          if self.decls[*index] == *property {
             return true;
           }
-          *prev = custom.clone();
+          let mut custom = custom.clone();
+          self.add_conditional_fallbacks(&mut custom, context);
+          self.decls[*index] = Property::Custom(custom);
         } else {
-          self.custom_properties.insert(name.clone(), custom.clone());
+          self.custom_properties.insert(name.clone(), self.decls.len());
+          let mut custom = custom.clone();
+          self.add_conditional_fallbacks(&mut custom, context);
+          self.decls.push(Property::Custom(custom));
         }
 
         return true;
@@ -600,13 +604,17 @@ impl<'i> DeclarationHandler<'i> {
         true
       }
       Property::All(keyword) => {
-        *self = DeclarationHandler {
-          custom_properties: std::mem::take(&mut self.custom_properties),
+        let mut handler = DeclarationHandler {
           unicode_bidi: self.unicode_bidi.clone(),
           direction: self.direction.clone(),
-          all: Some(keyword.clone()),
           ..Default::default()
         };
+        for (key, index) in self.custom_properties.drain() {
+          handler.custom_properties.insert(key, handler.decls.len());
+          handler.decls.push(self.decls[index].clone());
+        }
+        handler.decls.push(Property::All(keyword.clone()));
+        *self = handler;
         true
       }
       _ => false,
@@ -633,20 +641,12 @@ impl<'i> DeclarationHandler<'i> {
   }
 
   pub fn finalize(&mut self, context: &mut PropertyHandlerContext<'i, '_>) {
-    // Always place the `all` property first. Previous properties will have been omitted.
-    if let Some(all) = std::mem::take(&mut self.all) {
-      self.decls.push(Property::All(all));
-    }
     if let Some(direction) = std::mem::take(&mut self.direction) {
       self.decls.push(Property::Direction(direction));
     }
     if let Some(unicode_bidi) = std::mem::take(&mut self.unicode_bidi) {
       self.decls.push(Property::UnicodeBidi(unicode_bidi));
     }
-    for (_, mut value) in std::mem::take(&mut self.custom_properties) {
-      self.add_conditional_fallbacks(&mut value, context);
-      self.decls.push(Property::Custom(value));
-    }
 
     self.background.finalize(&mut self.decls, context);
     self.border.finalize(&mut self.decls, context);
@@ -675,5 +675,6 @@ impl<'i> DeclarationHandler<'i> {
     self.color_scheme.finalize(&mut self.decls, context);
     self.fallback.finalize(&mut self.decls, context);
     self.prefix.finalize(&mut self.decls, context);
+    self.custom_properties.clear();
   }
 }

src/declaration.rs

@@ -21583,26 +21583,26 @@ mod tests {
       indoc! {r#"
       @keyframes foo {
         from {
-          opacity: 0;
           --custom: #ff0;
+          opacity: 0;
         }
 
         to {
-          opacity: 1;
           --custom: #ee00be;
+          opacity: 1;
         }
       }
 
       @supports (color: lab(0% 0 0)) {
         @keyframes foo {
           from {
-            opacity: 0;
             --custom: #ff0;
+            opacity: 0;
           }
 
           to {
-            opacity: 1;
             --custom: lab(50.998% 125.506 -50.7078);
+            opacity: 1;
           }
         }
       }
@@ -23736,8 +23736,8 @@ mod tests {
       }
 
       .EgL3uq_foo {
-        color: var(--foo);
         --foo: red;
+        color: var(--foo);
       }
     "#},
       map! {
@@ -23786,10 +23786,10 @@ mod tests {
       }
 
       .EgL3uq_foo {
-        color: var(--EgL3uq_foo);
-        font-palette: --EgL3uq_Cooler;
         --EgL3uq_foo: red;
         --EgL3uq_bar: green;
+        color: var(--EgL3uq_foo);
+        font-palette: --EgL3uq_Cooler;
       }
 
       .EgL3uq_bar {
@@ -27646,7 +27646,7 @@ mod tests {
     );
     minify_test(
       ".foo { --test: red; all: revert-layer }",
-      ".foo{all:revert-layer;--test:red}",
+      ".foo{--test:red;all:revert-layer}",
     );
     minify_test(
       ".foo { unicode-bidi: embed; all: revert-layer }",
@@ -27660,5 +27660,10 @@ mod tests {
       ".foo { direction: rtl; all: revert-layer; direction: ltr }",
       ".foo{all:revert-layer;direction:ltr}",
     );
+    minify_test(".foo { background: var(--foo); all: unset; }", ".foo{all:unset}");
+    minify_test(
+      ".foo { all: unset; background: var(--foo); }",
+      ".foo{all:unset;background:var(--foo)}",
+    );
   }
 }