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(property setup): original property being mutated if the value is an object #534

Merged
merged 1 commit into from
Feb 4, 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
26 changes: 26 additions & 0 deletions __tests__/exportPlatform.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,32 @@ describe('exportPlatform', () => {
expect(dictionary.color.font.link.original.value).toEqual(properties.color.font.link.value);
});

it('should not mutate original value if value is an object', () => {
const dictionary = StyleDictionary.extend({
properties: {
color: {
red: {
value: {
h: "{hue.red}",
s: 50,
l: 50
}
}
},
hue: {
red: 20
}
},
platforms: {
web: {
transformGroup: 'web'
}
}
}).exportPlatform('web');
expect(dictionary.color.red.original.value.h).toEqual("{hue.red}");
expect(dictionary.color.red.value.h).toEqual(20);
});

describe('reference warnings', () => {
const errorMessage = `Problems were found when trying to resolve property references`;
const platforms = {
Expand Down
10 changes: 10 additions & 0 deletions __tests__/transform/propertySetup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,15 @@ describe('transform', () => {
expect(test).toHaveProperty('name', 'white');
});

it('should handle objects', () => {
const test = propertySetup({
value: {
h: 20, s: 50, l: 50
}
}, 'red', ['color','red']);
expect(test).toHaveProperty('value.h', 20);
expect(test).toHaveProperty('original.value.h', 20);
})

});
});
8 changes: 5 additions & 3 deletions lib/transform/propertySetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
* and limitations under the License.
*/

var _ = require('lodash');
const _ = require('lodash');
const deepExtend = require('../utils/deepExtend');

/**
* Takes a property object, a leaf node in a properties object, and
Expand All @@ -32,14 +33,15 @@ function propertySetup(property, name, path) {
if (!path || !_.isArray(path))
throw new Error('Path must be an array');

let to_ret = _.clone(property);
let to_ret = property;

// Only do this once
if (!property.original) {
// Initial property setup
// Keep the original object properties like it was in file (whitout additional data)
// so we can key off them in the transforms
var to_ret_original = _.clone(property);
to_ret = deepExtend([{}, property]);
let to_ret_original = deepExtend([{},property]);
delete to_ret_original.filePath;
delete to_ret_original.isSource;

Expand Down