Split tests for use-before-create and reusing mixin names for variables

- Reuse `valueProperty` boolean in apply shim
Polymer · Jul 28, 2016 · 8de1bec · 8de1bec
1 parent 6265ade
commit 8de1bec
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 14 deletions.
diff --git a/src/lib/apply-shim.html b/src/lib/apply-shim.html
@@ -133,20 +133,11 @@
 
   function produceCssProperties(matchText, propertyName, valueProperty, valueMixin) {
     // handle case where property value is a mixin
-    var possibleVariableRedefine = false;
     if (valueProperty) {
       // form: --mixin2: var(--mixin1), where --mixin1 is in the map
       styleUtil.processVariableAndFallback(valueProperty, function(prefix, value) {
         if (value && mapGet(value)) {
           valueMixin = '@apply ' + value + ';';
-          // because the mixinMap is global, we mark this rule
-          // as we might conflict with a different scope's simple variable
-          // Example:
-          // some style somewhere:
-          // --foo:{ ... }
-          // some other element:
-          // --foo: red;
-          possibleVariableRedefine = true;
         }
       });
     }
@@ -188,7 +179,17 @@
     if (mixinEntry) {
       mixinEntry.properties = combinedProps;
     }
-    if (possibleVariableRedefine) {
+    // because the mixinMap is global, the mixin might conflict with
+    // a different scope's simple variable definition:
+    // Example:
+    // some style somewhere:
+    // --mixin1:{ ... }
+    // --mixin2: var(--mixin1);
+    // some other element:
+    // --mixin1: 10px solid red;
+    // --foo: var(--mixin1);
+    // In this case, we leave the original variable definition in place.
+    if (valueProperty) {
       prefix = matchText + ';' + prefix;
     }
     return prefix + out.join('; ') + ';';

diff --git a/test/unit/styling-cross-scope-apply.html b/test/unit/styling-cross-scope-apply.html
@@ -537,7 +537,6 @@
   <template>
     <style>
     :host {
-      color: var(--color);
       @apply --before-create;
     }
     </style>
@@ -558,8 +557,6 @@
       --before-create: {
         border: 2px solid green;
       };
-      --foo: rgb(0, 0, 255);
-      --color: var(--foo);
     }
     </style>
     <x-apply-depend-before-create id="child"></x-apply-depend-before-create>
@@ -573,6 +570,27 @@
   </script>
 </dom-module>
 
+<dom-module id="x-reuse-mixin-name-for-variable">
+  <template>
+    <style>
+    :host {
+      /* --foo is used above as a mixin for color */
+      --foo: 20px solid blue;
+      /* this looks suspiciously like defining a mixin from another mixin */
+      --color: var(--foo);
+      border: var(--color);
+    }
+    </style>
+  </template>
+  <script>
+  HTMLImports.whenReady(function() {
+    Polymer({
+      is: 'x-reuse-mixin-name-for-variable'
+    })
+  });
+  </script>
+</dom-module>
+
 <script>
 suite('scoped-styling-apply', function() {
   function assertComputed(element, value, property) {
@@ -785,7 +803,13 @@
     document.body.appendChild(e2);
     CustomElements.takeRecords();
     assertComputed(e2.$.child, '2px');
-    assertComputed(e2.$.child, 'rgb(0, 0, 255)', 'color');
+  });
+
+  test('mixin names can be safely reused for variable definitions', function() {
+    var e = document.createElement('x-reuse-mixin-name-for-variable');
+    document.body.appendChild(e);
+    CustomElements.takeRecords();
+    assertComputed(e, '20px');
   });
 });