Skip to content

Commit

Permalink
Make sure we don't populate the mixin map for every variable
Browse files Browse the repository at this point in the history
Also, ensure that the form `--var1: var(--var2)`, used for mixin
aliasing, does not break variable definition in the case that `--var2`
is a reused name.
  • Loading branch information
dfreedm committed Jul 27, 2016
1 parent 2cab461 commit 6265ade
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
27 changes: 20 additions & 7 deletions src/lib/apply-shim.html
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,7 @@

function mapGet(name) {
name = name.trim();
var entry = mixinMap[name];
// if the mixin is not defined, make an entry so we can set up dependencies correctly
if (!entry) {
mapSet(name, {});
entry = mixinMap[name];
}
return entry;
return mixinMap[name];
}

// "parse" a mixin definition into a map of properties and values
Expand Down Expand Up @@ -139,10 +133,20 @@

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;
}
});
}
Expand Down Expand Up @@ -184,6 +188,9 @@
if (mixinEntry) {
mixinEntry.properties = combinedProps;
}
if (possibleVariableRedefine) {
prefix = matchText + ';' + prefix;
}
return prefix + out.join('; ') + ';';
}

Expand All @@ -202,6 +209,12 @@
mixinName = mixinName.replace(APPLY_NAME_CLEAN, '');
var vars = [];
var mixinEntry = mapGet(mixinName);
// if we depend on a mixin before it is created
// make a sentinel entry in the map to add this element as a dependency for when it is defined.
if (!mixinEntry) {
mapSet(mixinName, {});
mixinEntry = mapGet(mixinName);
}
if (mixinEntry) {
var currentProto = ApplyShim.__currentElementProto;
if (currentProto) {
Expand Down
7 changes: 6 additions & 1 deletion test/unit/styling-cross-scope-apply.html
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@
<template>
<style>
:host {
color: var(--color);
@apply --before-create;
}
</style>
Expand All @@ -556,7 +557,9 @@
:host {
--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>
Expand Down Expand Up @@ -777,10 +780,12 @@
test('mixins can be depended on before creation', function() {
var e = document.createElement('x-apply-depend-before-create');
document.body.appendChild(e);
CustomElements.takeRecords();
var e2 = document.createElement('x-create-after-apply');
document.body.appendChild(e2);
CustomElements.takeRecords();
assertComputed(e2.$.child, '2px');
assertComputed(e2.$.child, 'rgb(0, 0, 255)', 'color');
});
});

Expand Down

0 comments on commit 6265ade

Please sign in to comment.