Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(export platform): fixing infinite loop when there are reference errors #531

Merged
merged 2 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
chazzmoney marked this conversation as resolved.
Show resolved Hide resolved
}
stack.pop(variable);

Expand Down