Skip to content

Commit

Permalink
fix(export platform): fixing infinite loop when there are reference e…
Browse files Browse the repository at this point in the history
…rrors (#531)

When there were property reference errors (could not find reference or circular reference) Style Dictionary would get caught in an infinite loop. To fix this, when Style Dictionary tries to resolve an erroneous reference (circular or not found), it replaces the reference with `undefined`. This is different than previous behavior where Style Dictionary would leave the erroneous reference. In both cases, Style Dictionary throws an error and fails, so this should not have a discernible effect. We also feel replacing the reference with `undefined` makes logical sense as that reference is undefined because it cannot be resolved.
  • Loading branch information
dbanksdesign authored Feb 2, 2021
1 parent 7e7889a commit 6078c80
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 19 deletions.
97 changes: 80 additions & 17 deletions __tests__/exportPlatform.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,59 +11,60 @@
* and limitations under the License.
*/

var helpers = require('./__helpers');
var keys = require('lodash/keys');
var config = helpers.fileToJSON(__dirname + '/__configs/test.json');
var StyleDictionary = require('../index').extend(config);
const helpers = require('./__helpers');
const keys = require('lodash/keys');
const config = helpers.fileToJSON(__dirname + '/__configs/test.json');
const StyleDictionary = require('../index');
const styleDictionary = StyleDictionary.extend(config);

describe('exportPlatform', () => {

it('should throw if not given a platform', () => {
expect(
function(){
StyleDictionary.exportPlatform()
styleDictionary.exportPlatform()
}
).toThrow();
});

it('should throw if not given a proper platform', () => {
expect(
function(){
StyleDictionary.exportPlatform('foo');
styleDictionary.exportPlatform('foo');
}
).toThrow();
});

it('should not throw if given a proper platform', () => {
expect(
function(){
StyleDictionary.exportPlatform('web');
styleDictionary.exportPlatform('web');
}
).not.toThrow();
});

it('should return an object', () => {
var dictionary = StyleDictionary.exportPlatform('web');
var dictionary = styleDictionary.exportPlatform('web');
expect(typeof dictionary).toBe('object');
});

it('should have the same structure as the original properties', () => {
var dictionary = StyleDictionary.exportPlatform('web');
expect(keys(dictionary)).toEqual(keys(StyleDictionary.properties));
var dictionary = styleDictionary.exportPlatform('web');
expect(keys(dictionary)).toEqual(keys(styleDictionary.properties));
});

it('should have resolved references', () => {
var dictionary = StyleDictionary.exportPlatform('web');
var dictionary = styleDictionary.exportPlatform('web');
expect(dictionary.color.font.link.value).toEqual(dictionary.color.base.blue['100'].value);
});

it('should have applied transforms', () => {
var dictionary = StyleDictionary.exportPlatform('web');
var dictionary = styleDictionary.exportPlatform('web');
expect(dictionary.size.padding.base.value.indexOf('px')).toBeGreaterThan(0);
});

it('should have applied transforms for props with refs in it', () => {
const StyleDictionaryExtended = StyleDictionary.extend({
const StyleDictionaryExtended = styleDictionary.extend({
platforms: {
test: {
transforms: ['color/css','color/darken']
Expand All @@ -86,17 +87,79 @@ describe('exportPlatform', () => {
});

it('should not have mutated the original properties', () => {
var dictionary = StyleDictionary.exportPlatform('web');
expect(dictionary.color.font.link.value).not.toEqual(StyleDictionary.properties.color.font.link.value);
expect(StyleDictionary.properties.size.padding.base.value.indexOf('px')).toBe(-1);
var dictionary = styleDictionary.exportPlatform('web');
expect(dictionary.color.font.link.value).not.toEqual(styleDictionary.properties.color.font.link.value);
expect(styleDictionary.properties.size.padding.base.value.indexOf('px')).toBe(-1);
});

// Make sure when we perform transforms and resolve references
// we don't mutate the original object added to the property.
it('properties should have original value untouched', () => {
var dictionary = StyleDictionary.exportPlatform('web');
var dictionary = styleDictionary.exportPlatform('web');
var properties = helpers.fileToJSON(__dirname + '/__properties/colors.json');
expect(dictionary.color.font.link.original.value).toEqual(properties.color.font.link.value);
});

describe('reference warnings', () => {
const errorMessage = `Problems were found when trying to resolve property references`;
const platforms = {
css: {
transformGroup: `css`
}
}

it('should throw if there are simple property reference errors', () => {
const properties = {
a: "#ff0000",
b: "{c}"
}
expect(
function() {
StyleDictionary.extend({
properties,
platforms
}).exportPlatform('css')
}
).toThrow(errorMessage);
});

it('should throw if there are circular reference errors', () => {
const properties = {
a: "{b}",
b: "{a}"
}
expect(
function() {
StyleDictionary.extend({
properties,
platforms
}).exportPlatform('css')
}
).toThrow(errorMessage);
});

it('should throw if there are complex property reference errors', () => {
const properties = {
color: {
core: {
red: { valuer: "#ff0000" }, // notice misspelling
blue: { "value:": "#0000ff" }
},
danger: { value: "{color.core.red.value}" },
warning: { value: "{color.base.red.valuer}" },
info: { value: "{color.core.blue.value}" },
error: { value: "{color.danger.value}" }
}
}
expect(
function() {
StyleDictionary.extend({
properties,
platforms
}).exportPlatform('css')
}
).toThrow(errorMessage);
});
});

});
3 changes: 1 addition & 2 deletions lib/utils/resolveObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ function compile_value(value, stack) {
PROPERTY_REFERENCE_WARNINGS,
"Reference doesn't exist: " + context + " tries to reference " + variable + ", which is not defined"
);
// Leave the circular reference unchanged
to_ret = value;
to_ret = ref;
}
stack.pop(variable);

Expand Down

0 comments on commit 6078c80

Please sign in to comment.