From c2e6c923154925ced4ea71ceeb650f23dc83c025 Mon Sep 17 00:00:00 2001 From: Danny Banks Date: Mon, 1 Mar 2021 16:50:21 -0800 Subject: [PATCH 1/5] fix(references): use unfiltered dictionary for reference resolution in formats --- .../__snapshots__/customFormats.test.js.snap | 336 +++++++++++++++++- .../__snapshots__/scss.test.js.snap | 15 + __integration__/scss.test.js | 14 + lib/buildFile.js | 10 +- lib/filterProperties.js | 4 +- lib/utils/references/getReference.js | 3 +- 6 files changed, 373 insertions(+), 9 deletions(-) diff --git a/__integration__/__snapshots__/customFormats.test.js.snap b/__integration__/__snapshots__/customFormats.test.js.snap index db6897af7..2db8996df 100644 --- a/__integration__/__snapshots__/customFormats.test.js.snap +++ b/__integration__/__snapshots__/customFormats.test.js.snap @@ -162,7 +162,89 @@ exports[`integration custom formats inline custom with new args should match sna \\"xl\\" ] } - ] + ], + \\"_properties\\": { + \\"size\\": { + \\"padding\\": { + \\"small\\": { + \\"value\\": \\"0.5rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 0.5 + }, + \\"name\\": \\"SizePaddingSmall\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"small\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"small\\" + ] + }, + \\"medium\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingMedium\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"medium\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"medium\\" + ] + }, + \\"large\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingLarge\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"large\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"large\\" + ] + }, + \\"xl\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingXl\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"xl\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"xl\\" + ] + } + } + } + } }, \\"allProperties\\": [ { @@ -539,7 +621,89 @@ exports[`integration custom formats inline custom with old args should match sna \\"xl\\" ] } - ] + ], + \\"_properties\\": { + \\"size\\": { + \\"padding\\": { + \\"small\\": { + \\"value\\": \\"0.5rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 0.5 + }, + \\"name\\": \\"SizePaddingSmall\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"small\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"small\\" + ] + }, + \\"medium\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingMedium\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"medium\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"medium\\" + ] + }, + \\"large\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingLarge\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"large\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"large\\" + ] + }, + \\"xl\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingXl\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"xl\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"xl\\" + ] + } + } + } + } }, \\"allProperties\\": [ { @@ -961,7 +1125,89 @@ exports[`integration custom formats register custom format with new args should \\"xl\\" ] } - ] + ], + \\"_properties\\": { + \\"size\\": { + \\"padding\\": { + \\"small\\": { + \\"value\\": \\"0.5rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 0.5 + }, + \\"name\\": \\"SizePaddingSmall\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"small\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"small\\" + ] + }, + \\"medium\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingMedium\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"medium\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"medium\\" + ] + }, + \\"large\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingLarge\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"large\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"large\\" + ] + }, + \\"xl\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingXl\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"xl\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"xl\\" + ] + } + } + } + } }, \\"allProperties\\": [ { @@ -1338,7 +1584,89 @@ exports[`integration custom formats register custom format with old args should \\"xl\\" ] } - ] + ], + \\"_properties\\": { + \\"size\\": { + \\"padding\\": { + \\"small\\": { + \\"value\\": \\"0.5rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 0.5 + }, + \\"name\\": \\"SizePaddingSmall\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"small\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"small\\" + ] + }, + \\"medium\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingMedium\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"medium\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"medium\\" + ] + }, + \\"large\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingLarge\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"large\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"large\\" + ] + }, + \\"xl\\": { + \\"value\\": \\"1rem\\", + \\"filePath\\": \\"__integration__/tokens/size/padding.json\\", + \\"isSource\\": true, + \\"original\\": { + \\"value\\": 1 + }, + \\"name\\": \\"SizePaddingXl\\", + \\"attributes\\": { + \\"category\\": \\"size\\", + \\"type\\": \\"padding\\", + \\"item\\": \\"xl\\" + }, + \\"path\\": [ + \\"size\\", + \\"padding\\", + \\"xl\\" + ] + } + } + } + } }, \\"allProperties\\": [ { diff --git a/__integration__/__snapshots__/scss.test.js.snap b/__integration__/__snapshots__/scss.test.js.snap index 9c3cc88b1..fe7248e4c 100644 --- a/__integration__/__snapshots__/scss.test.js.snap +++ b/__integration__/__snapshots__/scss.test.js.snap @@ -732,6 +732,21 @@ $size-padding-large: 1rem; $size-padding-xl: 1rem;" `; +exports[`integration scss scss/variables with filter and output references should match snapshot 1`] = ` +" +// Do not edit directly +// Generated on Sat, 01 Jan 2000 00:00:00 GMT + +$color-background-primary: $color-core-neutral-0 !default; +$color-background-secondary: $color-core-neutral-100; +$color-background-tertiary: $color-core-neutral-200; +$color-background-danger: $color-core-red-0; +$color-background-warning: $color-core-orange-0; +$color-background-success: $color-core-green-0; +$color-background-info: $color-core-blue-0; +$color-background-disabled: $color-background-tertiary;" +`; + exports[`integration scss scss/variables with outputReferences should match snapshot 1`] = ` " // Do not edit directly diff --git a/__integration__/scss.test.js b/__integration__/scss.test.js index 580e81299..d272563b7 100644 --- a/__integration__/scss.test.js +++ b/__integration__/scss.test.js @@ -33,6 +33,13 @@ describe(`integration`, () => { options: { outputReferences: true } + },{ + destination: `filteredVariablesWithReferences.scss`, + format: `scss/variables`, + filter: (token) => token.path[1] === 'background', + options: { + outputReferences: true + } },{ destination: `map-flat.scss`, format: `scss/map-flat` @@ -71,6 +78,13 @@ describe(`integration`, () => { expect(output).toMatchSnapshot(); }); }); + + describe(`with filter and output references`, () => { + const output = fs.readFileSync(`${buildPath}filteredVariablesWithReferences.scss`, {encoding:'UTF-8'}); + it(`should match snapshot`, () => { + expect(output).toMatchSnapshot(); + }); + }); }); describe(`scss/map-flat`, () => { diff --git a/lib/buildFile.js b/lib/buildFile.js index 5a7ca8f0d..e9f7b5f58 100644 --- a/lib/buildFile.js +++ b/lib/buildFile.js @@ -49,7 +49,13 @@ function buildFile(file = {}, platform = {}, dictionary = {}) { if (!fs.existsSync(dirname)) fs.mkdirsSync(dirname); - var filteredProperties = filterProperties(dictionary, filter); + const filteredProperties = filterProperties(dictionary, filter); + const filteredDictionary = Object.assign({}, dictionary, { + properties: filteredProperties.properties, + allProperties: filteredProperties.allProperties, + // keep the unfiltered properties object for reference resolution + _properties: dictionary.properties + }); // if properties object is empty, return without creating a file if ( @@ -90,7 +96,7 @@ function buildFile(file = {}, platform = {}, dictionary = {}) { let propertyNamesCollisionCount = GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS); fs.writeFileSync(fullDestination, format(createFormatArgs({ - dictionary: filteredProperties, + dictionary: filteredDictionary, platform, file }), platform, file)); diff --git a/lib/filterProperties.js b/lib/filterProperties.js index db24c9704..346f0b398 100644 --- a/lib/filterProperties.js +++ b/lib/filterProperties.js @@ -80,10 +80,10 @@ function filterProperties(dictionary, filter) { if (!filter) { return dictionary } else { - return _.assign({}, dictionary, { + return { allProperties: filterPropertyArray(dictionary.allProperties, filter), properties: filterPropertyObject(dictionary.properties, filter) - }) + }; } } diff --git a/lib/utils/references/getReference.js b/lib/utils/references/getReference.js index e60e57875..70d01b3f8 100644 --- a/lib/utils/references/getReference.js +++ b/lib/utils/references/getReference.js @@ -41,7 +41,8 @@ function getReference(value) { // Find what the value is referencing const pathName = getPath(variable); - ref = resolveReference(pathName, self.properties); + // using _properties as it is unfiltered + ref = resolveReference(pathName, self._properties); }); return ref; From a6b4cc7808e7e55df19c97406f2a9c5747b79dfa Mon Sep 17 00:00:00 2001 From: Danny Banks Date: Fri, 5 Mar 2021 16:09:01 -0800 Subject: [PATCH 2/5] fix(extend): remove prototype pollution (#554) --- __tests__/extend.test.js | 9 +++++++++ lib/utils/deepExtend.js | 2 ++ 2 files changed, 11 insertions(+) diff --git a/__tests__/extend.test.js b/__tests__/extend.test.js index 1c50bd6e8..30693f7c6 100644 --- a/__tests__/extend.test.js +++ b/__tests__/extend.test.js @@ -245,4 +245,13 @@ describe('extend', () => { expect(StyleDictionary3.foo).toBe('boo'); expect(StyleDictionary).not.toHaveProperty('foo'); }); + + it(`should not pollute the prototype`, () => { + const obj = {}; + let opts = JSON.parse('{"__proto__":{"polluted":"yes"}}'); + console.log("Before : " + obj.polluted); + StyleDictionary.extend(opts); + console.log("After : " + obj.polluted); + expect(obj.polluted).toBeUndefined(); + }); }); diff --git a/lib/utils/deepExtend.js b/lib/utils/deepExtend.js index 6f7555227..9c81bc10e 100644 --- a/lib/utils/deepExtend.js +++ b/lib/utils/deepExtend.js @@ -44,6 +44,8 @@ function deepExtend(objects, collision, path) { for (name in options) { if (!options.hasOwnProperty(name)) continue; + if (name === '__proto__') + continue; src = target[name]; copy = options[name]; From 72d10c2c5656d7a8e581e8f8ca99f0d84c7f6c80 Mon Sep 17 00:00:00 2001 From: Danny Banks Date: Tue, 9 Mar 2021 12:42:26 -0800 Subject: [PATCH 3/5] chore(release): 3.0.0-rc.6 --- CHANGELOG.md | 7 +++++++ examples/advanced/assets-base64-embed/package.json | 2 +- examples/advanced/auto-rebuild-watcher/package.json | 2 +- examples/advanced/component-cti/package.json | 2 +- examples/advanced/create-react-app/package.json | 2 +- examples/advanced/create-react-native-app/package.json | 2 +- .../advanced/custom-formats-with-templates/package.json | 2 +- examples/advanced/custom-parser/package.json | 2 +- examples/advanced/custom-transforms/package.json | 2 +- examples/advanced/matching-build-files/package.json | 2 +- examples/advanced/multi-brand-multi-platform/package.json | 2 +- .../node-modules-as-config-and-properties/package.json | 2 +- examples/advanced/npm-module/package.json | 2 +- examples/advanced/referencing_aliasing/package.json | 2 +- examples/advanced/s3/package.json | 2 +- examples/advanced/tokens-deprecation/package.json | 2 +- examples/advanced/transitive-transforms/package.json | 2 +- examples/advanced/variables-in-outputs/package.json | 2 +- examples/advanced/yaml-tokens/package.json | 2 +- package-lock.json | 2 +- package.json | 2 +- 21 files changed, 27 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 075bbf123..270e5c0ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines. +## [3.0.0-rc.6](https://github.com/amzn/style-dictionary/compare/v3.0.0-rc.5...v3.0.0-rc.6) (2021-03-09) + + +### Bug Fixes + +* **extend:** remove prototype pollution ([#554](https://github.com/amzn/style-dictionary/issues/554)) ([b99710a](https://github.com/amzn/style-dictionary/commit/b99710a23abf7d49be28f4ce33dbe99a8af5923f)) + ## [3.0.0-rc.5](https://github.com/amzn/style-dictionary/compare/v3.0.0-rc.4...v3.0.0-rc.5) (2021-02-27) diff --git a/examples/advanced/assets-base64-embed/package.json b/examples/advanced/assets-base64-embed/package.json index d3621449c..d22906009 100644 --- a/examples/advanced/assets-base64-embed/package.json +++ b/examples/advanced/assets-base64-embed/package.json @@ -11,6 +11,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/auto-rebuild-watcher/package.json b/examples/advanced/auto-rebuild-watcher/package.json index a972881c7..bd01b322a 100644 --- a/examples/advanced/auto-rebuild-watcher/package.json +++ b/examples/advanced/auto-rebuild-watcher/package.json @@ -17,6 +17,6 @@ "license": "Apache-2.0", "devDependencies": { "chokidar-cli": "^1.2.0", - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/component-cti/package.json b/examples/advanced/component-cti/package.json index 2ab1b8c37..c43a39ac0 100644 --- a/examples/advanced/component-cti/package.json +++ b/examples/advanced/component-cti/package.json @@ -15,6 +15,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/create-react-app/package.json b/examples/advanced/create-react-app/package.json index 771967205..c77667e72 100644 --- a/examples/advanced/create-react-app/package.json +++ b/examples/advanced/create-react-app/package.json @@ -10,7 +10,7 @@ "styled-components": "^5.1.1" }, "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" }, "scripts": { "build-dictionary": "style-dictionary build --config ./style-dictionary/config.json", diff --git a/examples/advanced/create-react-native-app/package.json b/examples/advanced/create-react-native-app/package.json index 965d70411..ff89d8b34 100644 --- a/examples/advanced/create-react-native-app/package.json +++ b/examples/advanced/create-react-native-app/package.json @@ -27,7 +27,7 @@ "babel-jest": "~25.2.6", "jest": "~25.2.6", "react-test-renderer": "~16.13.1", - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" }, "jest": { "preset": "react-native" diff --git a/examples/advanced/custom-formats-with-templates/package.json b/examples/advanced/custom-formats-with-templates/package.json index 11f7c0f0e..afd45d8b2 100644 --- a/examples/advanced/custom-formats-with-templates/package.json +++ b/examples/advanced/custom-formats-with-templates/package.json @@ -19,6 +19,6 @@ "handlebars": "^4.0.12", "lodash": "^4.17.11", "pug": "^2.0.3", - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/custom-parser/package.json b/examples/advanced/custom-parser/package.json index ac91e35ee..d7d0fda12 100644 --- a/examples/advanced/custom-parser/package.json +++ b/examples/advanced/custom-parser/package.json @@ -9,6 +9,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/custom-transforms/package.json b/examples/advanced/custom-transforms/package.json index a5c9e91fa..9f36d0edc 100644 --- a/examples/advanced/custom-transforms/package.json +++ b/examples/advanced/custom-transforms/package.json @@ -15,6 +15,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/matching-build-files/package.json b/examples/advanced/matching-build-files/package.json index f8ce35b53..7e7ae1df9 100644 --- a/examples/advanced/matching-build-files/package.json +++ b/examples/advanced/matching-build-files/package.json @@ -16,6 +16,6 @@ "author": "Kelly Harrop ", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/multi-brand-multi-platform/package.json b/examples/advanced/multi-brand-multi-platform/package.json index 25b3159b0..694701fb0 100644 --- a/examples/advanced/multi-brand-multi-platform/package.json +++ b/examples/advanced/multi-brand-multi-platform/package.json @@ -15,6 +15,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/node-modules-as-config-and-properties/package.json b/examples/advanced/node-modules-as-config-and-properties/package.json index 509914550..85319aa65 100644 --- a/examples/advanced/node-modules-as-config-and-properties/package.json +++ b/examples/advanced/node-modules-as-config-and-properties/package.json @@ -19,7 +19,7 @@ }, "homepage": "https://github.com/dbanksdesign/style-dictionary-node#readme", "devDependencies": { - "style-dictionary": "3.0.0-rc.5", + "style-dictionary": "3.0.0-rc.6", "tinycolor2": "^1.4.1" } } \ No newline at end of file diff --git a/examples/advanced/npm-module/package.json b/examples/advanced/npm-module/package.json index 6d3b49f66..42c09f23d 100644 --- a/examples/advanced/npm-module/package.json +++ b/examples/advanced/npm-module/package.json @@ -16,6 +16,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/referencing_aliasing/package.json b/examples/advanced/referencing_aliasing/package.json index f44f8dff8..9be143dec 100644 --- a/examples/advanced/referencing_aliasing/package.json +++ b/examples/advanced/referencing_aliasing/package.json @@ -15,6 +15,6 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/s3/package.json b/examples/advanced/s3/package.json index ce0a44e04..6bcec6eba 100644 --- a/examples/advanced/s3/package.json +++ b/examples/advanced/s3/package.json @@ -15,6 +15,6 @@ "devDependencies": { "aws-sdk": "^2.7.21", "fs-extra": "^1.0.0", - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/tokens-deprecation/package.json b/examples/advanced/tokens-deprecation/package.json index 65559e95f..7685e5b26 100644 --- a/examples/advanced/tokens-deprecation/package.json +++ b/examples/advanced/tokens-deprecation/package.json @@ -16,6 +16,6 @@ "license": "Apache-2.0", "devDependencies": { "lodash": "^4.17.11", - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/transitive-transforms/package.json b/examples/advanced/transitive-transforms/package.json index f6d2a04d3..3fed04f44 100644 --- a/examples/advanced/transitive-transforms/package.json +++ b/examples/advanced/transitive-transforms/package.json @@ -11,6 +11,6 @@ "license": "ISC", "devDependencies": { "chroma-js": "^2.1.0", - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/variables-in-outputs/package.json b/examples/advanced/variables-in-outputs/package.json index 372f7b097..a384cfb64 100644 --- a/examples/advanced/variables-in-outputs/package.json +++ b/examples/advanced/variables-in-outputs/package.json @@ -10,6 +10,6 @@ "author": "", "license": "MIT", "devDependencies": { - "style-dictionary": "3.0.0-rc.5" + "style-dictionary": "3.0.0-rc.6" } } \ No newline at end of file diff --git a/examples/advanced/yaml-tokens/package.json b/examples/advanced/yaml-tokens/package.json index 03421936e..1f3d67d43 100644 --- a/examples/advanced/yaml-tokens/package.json +++ b/examples/advanced/yaml-tokens/package.json @@ -9,7 +9,7 @@ "author": "", "license": "Apache-2.0", "devDependencies": { - "style-dictionary": "3.0.0-rc.5", + "style-dictionary": "3.0.0-rc.6", "yaml": "^1.10.0" } } \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 9ccefbe18..f52a06ae2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "style-dictionary", - "version": "3.0.0-rc.5", + "version": "3.0.0-rc.6", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 6b805a735..ea84dfb3b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "style-dictionary", - "version": "3.0.0-rc.5", + "version": "3.0.0-rc.6", "description": "Style once, use everywhere. A build system for creating cross-platform styles.", "keywords": [ "style dictionary", From 5777b622f465b061557a7c093afb2f4df38e047b Mon Sep 17 00:00:00 2001 From: Danny Banks Date: Fri, 12 Mar 2021 16:28:07 -0800 Subject: [PATCH 4/5] chore(logs): removing name collision warnings for nested formats (#567) --- __integration__/nameCollisions.test.js | 74 ++++++++++++++++++++++++++ __tests__/buildFile.test.js | 42 ++++++++++----- lib/buildFile.js | 13 +++-- lib/common/formats.js | 5 ++ 4 files changed, 117 insertions(+), 17 deletions(-) create mode 100644 __integration__/nameCollisions.test.js diff --git a/__integration__/nameCollisions.test.js b/__integration__/nameCollisions.test.js new file mode 100644 index 000000000..7cdf0fb0e --- /dev/null +++ b/__integration__/nameCollisions.test.js @@ -0,0 +1,74 @@ +/* + * Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with + * the License. A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + * CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions + * and limitations under the License. + */ + +const fs = require('fs-extra'); +const chalk = require('chalk'); +const StyleDictionary = require('../index'); +const {buildPath} = require('./_constants'); + +const properties = { + color: { + red: { value: '#f00' }, + background: { + red: { value: '{color.red.value}' } + } + } +} + +describe('integration', () => { + describe('name collisions', () => { + it(`should warn users of name collisions for flat files`, () => { + console.log = jest.fn(); + StyleDictionary.extend({ + // we are only testing name collision warnings options so we don't need + // the full source. + properties, + platforms: { + web: { + buildPath, + files: [{ + destination: 'variables.css', + format: 'css/variables', + }] + }, + } + }).buildAllPlatforms(); + expect(console.log).toHaveBeenCalledWith(`⚠️ ${buildPath}variables.css`); + }); + + it(`should not warn users of name collisions for nested files`, () => { + console.log = jest.fn(); + StyleDictionary.extend({ + // we are only testing name collision warnings options so we don't need + // the full source. + properties, + platforms: { + web: { + buildPath, + files: [{ + destination: 'tokens.json', + format: 'json/nested' + }] + }, + } + }).buildAllPlatforms(); + expect(console.log).toHaveBeenCalledWith(chalk.bold.green(`✔︎ ${buildPath}tokens.json`)); + }); + + + }); +}); + +afterAll(() => { + fs.emptyDirSync(buildPath); +}); \ No newline at end of file diff --git a/__tests__/buildFile.test.js b/__tests__/buildFile.test.js index 12e7fc54a..fb770ac31 100644 --- a/__tests__/buildFile.test.js +++ b/__tests__/buildFile.test.js @@ -20,6 +20,12 @@ function format() { return "hi"; } +function nestedFormat() { + return "hi"; +} + +nestedFormat.nested = true; + describe('buildFile', () => { beforeEach(() => { @@ -54,9 +60,9 @@ describe('buildFile', () => { ).toThrow('Please enter a valid destination'); }); - let destination = './__tests__/__output/test.collisions'; - var PROPERTY_NAME_COLLISION_WARNINGS = GroupMessages.GROUP.PropertyNameCollisionWarnings + ":" + destination; - it('should generate warning messages for output name collisions', () => { + describe('name collisions', () => { + let destination = './__tests__/__output/test.collisions'; + let PROPERTY_NAME_COLLISION_WARNINGS = GroupMessages.GROUP.PropertyNameCollisionWarnings + ":" + destination; let name = 'someName'; let properties = { allProperties: [{ @@ -69,22 +75,30 @@ describe('buildFile', () => { value: 'value2' }] }; + it('should generate warning messages for output name collisions', () => { + GroupMessages.clear(PROPERTY_NAME_COLLISION_WARNINGS); + buildFile({destination, format}, {}, properties); - GroupMessages.clear(PROPERTY_NAME_COLLISION_WARNINGS); - buildFile({destination, format}, {}, properties); + let collisions = properties.allProperties.map((properties) => { + let propertyPathText = chalk.keyword('orangered')(properties.path.join('.')); + let valueText = chalk.keyword('darkorange')(properties.value); + return propertyPathText + ' ' + valueText; + }).join('\n '); + let output = `Output name ${chalk.keyword('orangered').bold(name)} was generated by:\n ${collisions}`; + let expectJSON = JSON.stringify([output]); - let collisions = properties.allProperties.map((properties) => { - let propertyPathText = chalk.keyword('orangered')(properties.path.join('.')); - let valueText = chalk.keyword('darkorange')(properties.value); - return propertyPathText + ' ' + valueText; - }).join('\n '); - let output = `Output name ${chalk.keyword('orangered').bold(name)} was generated by:\n ${collisions}`; - let expectJSON = JSON.stringify([output]); + expect(GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS)).toBe(1); + expect(JSON.stringify(GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS))).toBe(expectJSON); + }); - expect(GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS)).toBe(1); - expect(JSON.stringify(GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS))).toBe(expectJSON); + it('should not warn users if the format is a nested format', () => { + console.log = jest.fn(); + buildFile({destination, format: nestedFormat}, {}, properties); + expect(console.log).toHaveBeenCalledWith(chalk.bold.green(`✔︎ ${destination}`)); + }); }); + let destEmptyProperties = './__tests__/__output/test.emptyProperties'; it('should warn when a file is not created because of empty properties', () => { let dictionary = { diff --git a/lib/buildFile.js b/lib/buildFile.js index e9f7b5f58..64ffa657c 100644 --- a/lib/buildFile.js +++ b/lib/buildFile.js @@ -36,7 +36,10 @@ function buildFile(file = {}, platform = {}, dictionary = {}) { if (typeof destination !== 'string') throw new Error('Please enter a valid destination'); - // to maintain backwards compatability we bind the format to the file object + // get if the format is nested, this needs to be done before + // the function is bound + const nested = format.nested; + // to maintain backwards compatibility we bind the format to the file object format = format.bind(file); var fullDestination = destination; @@ -100,9 +103,13 @@ function buildFile(file = {}, platform = {}, dictionary = {}) { platform, file }), platform, file)); - console.log((propertyNamesCollisionCount>0 ? '⚠️ ' : chalk.bold.green('✔︎ ')) + ' ' + fullDestination); - if(propertyNamesCollisionCount > 0) { + // don't show name collision warnings for nested type formats + // because they are not relevant. + if (nested || propertyNamesCollisionCount === 0) { + console.log( chalk.bold.green(`✔︎ ${fullDestination}`) ); + } else { + console.log( `⚠️ ${fullDestination}`); let propertyNamesCollisionWarnings = GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS).join('\n '); let title = `While building ${chalk.keyword('orangered').bold(destination)}, token collisions were found; output may be unexpected.`; let help = chalk.keyword('orange')([ diff --git a/lib/common/formats.js b/lib/common/formats.js index ec8f9d92e..2263353cd 100644 --- a/lib/common/formats.js +++ b/lib/common/formats.js @@ -879,3 +879,8 @@ module.exports = { return template({dictionary, file, options}); }, }; + +// Mark which formats are nested +module.exports['json/nested'].nested = true; +module.exports['javascript/module'].nested = true; +module.exports['javascript/object'].nested = true; \ No newline at end of file From 706cfcf8c3f02d1ec9e7c85b510c0bba1b2972f7 Mon Sep 17 00:00:00 2001 From: Danny Banks Date: Wed, 17 Mar 2021 21:53:31 -0700 Subject: [PATCH 5/5] chore(warnings): add warnings for filtered output references --- __integration__/outputReferences.test.js | 52 ++++++++++++++++++++++++ lib/buildFile.js | 37 ++++++++++++----- lib/utils/groupMessages.js | 1 + lib/utils/references/getReference.js | 12 +++++- 4 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 __integration__/outputReferences.test.js diff --git a/__integration__/outputReferences.test.js b/__integration__/outputReferences.test.js new file mode 100644 index 000000000..afa172b64 --- /dev/null +++ b/__integration__/outputReferences.test.js @@ -0,0 +1,52 @@ +/* + * Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with + * the License. A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR + * CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions + * and limitations under the License. + */ + +const fs = require('fs-extra'); +const StyleDictionary = require('../index'); +const {buildPath} = require('./_constants'); + +describe('integration', () => { + describe('output references', () => { + it('should warn the user if filters out references', () => { + console.log = jest.fn(); + StyleDictionary.extend({ + // we are only testing showFileHeader options so we don't need + // the full source. + source: [`__integration__/tokens/**/*.json`], + platforms: { + css: { + transformGroup: 'css', + buildPath, + files: [{ + destination: 'filteredVariables.css', + format: 'css/variables', + // filter tokens and use outputReferences + // Style Dictionary should build this file ok + // but warn the user + filter: (token) => token.attributes.type === 'background', + options: { + outputReferences: true + } + }] + } + } + }).buildAllPlatforms(); + + expect(console.log).toHaveBeenCalledWith(`⚠️ ${buildPath}filteredVariables.css`); + }); + }); +}); + +afterAll(() => { + fs.emptyDirSync(buildPath); +}); \ No newline at end of file diff --git a/lib/buildFile.js b/lib/buildFile.js index 64ffa657c..d7dbacf27 100644 --- a/lib/buildFile.js +++ b/lib/buildFile.js @@ -104,22 +104,37 @@ function buildFile(file = {}, platform = {}, dictionary = {}) { file }), platform, file)); + let filteredReferencesCount = GroupMessages.count(GroupMessages.GROUP.FilteredOutputReferences); + // console.log(GroupMessages.fetchMessages(GroupMessages.GROUP.FilteredOutputReferences).join('\n ')) + // don't show name collision warnings for nested type formats // because they are not relevant. - if (nested || propertyNamesCollisionCount === 0) { + if ((nested || propertyNamesCollisionCount === 0) && filteredReferencesCount === 0) { console.log( chalk.bold.green(`✔︎ ${fullDestination}`) ); } else { console.log( `⚠️ ${fullDestination}`); - let propertyNamesCollisionWarnings = GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS).join('\n '); - let title = `While building ${chalk.keyword('orangered').bold(destination)}, token collisions were found; output may be unexpected.`; - let help = chalk.keyword('orange')([ - 'This many-to-one issue is usually caused by some combination of:', - '* conflicting or similar paths/names in property definitions', - '* platform transforms/transformGroups affecting names, especially when removing specificity', - '* overly inclusive file filters', - ].join('\n ')); - let warn = `${title}\n ${propertyNamesCollisionWarnings}\n${help}`; - console.log(chalk.keyword('darkorange').bold(warn)); + if (propertyNamesCollisionCount > 0) { + let propertyNamesCollisionWarnings = GroupMessages.fetchMessages(PROPERTY_NAME_COLLISION_WARNINGS).join('\n '); + let title = `While building ${chalk.keyword('orangered').bold(destination)}, token collisions were found; output may be unexpected.`; + let help = chalk.keyword('orange')([ + 'This many-to-one issue is usually caused by some combination of:', + '* conflicting or similar paths/names in property definitions', + '* platform transforms/transformGroups affecting names, especially when removing specificity', + '* overly inclusive file filters', + ].join('\n ')); + let warn = `${title}\n ${propertyNamesCollisionWarnings}\n${help}`; + console.log(chalk.keyword('darkorange').bold(warn)); + } + + if (filteredReferencesCount > 0) { + let filteredReferencesWarnings = GroupMessages.fetchMessages(GroupMessages.GROUP.FilteredOutputReferences).join('\n '); + let title = `While building ${chalk.keyword('orangered').bold(destination)}, filtered out token references were found; output may be unexpected. Here are the references that are used but not defined in the file`; + let help = chalk.keyword('orange')([ + 'This is caused when combining a filter and `outputReferences`.', + ].join('\n ')); + let warn = `${title}\n ${filteredReferencesWarnings}\n${help}`; + console.log(chalk.keyword('darkorange').bold(warn)); + } } } diff --git a/lib/utils/groupMessages.js b/lib/utils/groupMessages.js index 78f2e5b35..33473a91d 100644 --- a/lib/utils/groupMessages.js +++ b/lib/utils/groupMessages.js @@ -21,6 +21,7 @@ var GroupMessages = { SassMapFormatDeprecationWarnings: 'Sass Map Format Deprecation Warnings', MissingRegisterTransformErrors: 'Missing Register Transform Errors', PropertyNameCollisionWarnings: 'Property Name Collision Warnings', + FilteredOutputReferences: 'Filtered Output Reference Warnings' }, flush: function (messageGroup) { diff --git a/lib/utils/references/getReference.js b/lib/utils/references/getReference.js index 70d01b3f8..f069c4977 100644 --- a/lib/utils/references/getReference.js +++ b/lib/utils/references/getReference.js @@ -14,6 +14,7 @@ const getPath = require('./getPathFromName'); const createReferenceRegex = require('./createReferenceRegex'); const resolveReference = require('./resolveReference'); +const GroupMessages = require('../groupMessages'); /** * This is a helper function that is added to the dictionary object that @@ -41,8 +42,15 @@ function getReference(value) { // Find what the value is referencing const pathName = getPath(variable); - // using _properties as it is unfiltered - ref = resolveReference(pathName, self._properties); + + ref = resolveReference(pathName, self.properties); + if (!ref) { + // fall back on _properties as it is unfiltered + ref = resolveReference(pathName, self._properties); + // and warn the user about this + GroupMessages.add(GroupMessages.GROUP.FilteredOutputReferences, variable); + } + }); return ref;