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

feat: allow not throwing on broken refs #1240

Merged
merged 3 commits into from
Jun 15, 2024
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
23 changes: 23 additions & 0 deletions .changeset/chilly-numbers-work.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'style-dictionary': minor
---

Allow not throwing fatal errors on broken token references/aliases, but `console.error` instead.

You can also configure this on global/platform `log` property:

```json
{
"log": {
"errors": {
"brokenReferences": "console"
}
}
}
```

This setting defaults to `"error"` when not configured.

`resolveReferences` and `getReferences` `warnImmediately` option is set to `true` which causes an error to be thrown/warned immediately by default, which can be configured to `false` if you know those utils are running in the transform/format hooks respectively, where the errors are collected and grouped, then thrown as 1 error/warning instead of multiple.

Some minor grammatical improvements to some of the error logs were also done.
20 changes: 16 additions & 4 deletions __integration__/__snapshots__/customFormats.test.snap.js
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,10 @@ snapshots["integration custom formats inline custom with new args should match s
],
"log": {
"warnings": "warn",
"verbosity": "default"
"verbosity": "default",
"errors": {
"brokenReferences": "throw"
}
},
"transforms": [
{
Expand Down Expand Up @@ -945,7 +948,10 @@ snapshots["integration custom formats inline custom with new args should match s
},
"log": {
"warnings": "warn",
"verbosity": "default"
"verbosity": "default",
"errors": {
"brokenReferences": "throw"
}
},
"usesDtcg": false,
"otherOption": "Test",
Expand Down Expand Up @@ -1503,7 +1509,10 @@ snapshots["integration custom formats register custom format with new args shoul
],
"log": {
"warnings": "warn",
"verbosity": "default"
"verbosity": "default",
"errors": {
"brokenReferences": "throw"
}
},
"transforms": [
{
Expand Down Expand Up @@ -1891,7 +1900,10 @@ snapshots["integration custom formats register custom format with new args shoul
},
"log": {
"warnings": "warn",
"verbosity": "default"
"verbosity": "default",
"errors": {
"brokenReferences": "throw"
}
},
"usesDtcg": false,
"otherOption": "Test",
Expand Down
6 changes: 6 additions & 0 deletions __integration__/logging/__snapshots__/file.test.snap.js
Original file line number Diff line number Diff line change
Expand Up @@ -439,3 +439,9 @@ color.core.blue.0
This is caused when combining a filter and \`outputReferences\`.`;
/* end snapshot integration logging file filtered references should throw detailed error of filtered references through "verbose" verbosity and log level set to error */

snapshots["integration logging file empty tokens should not warn user about empty tokens with silent log verbosity"] =
`
css
No tokens for empty.css. File not created.`;
/* end snapshot integration logging file empty tokens should not warn user about empty tokens with silent log verbosity */

124 changes: 123 additions & 1 deletion __tests__/StyleDictionary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ import { expect } from 'chai';
import StyleDictionary from 'style-dictionary';
import { fs } from 'style-dictionary/fs';
import chalk from 'chalk';
import { fileToJSON, clearOutput, fileExists } from './__helpers.js';
import { fileToJSON, clearOutput, fileExists, clearSDMeta } from './__helpers.js';
import { resolve } from '../lib/resolve.js';
import GroupMessages from '../lib/utils/groupMessages.js';
import flattenTokens from '../lib/utils/flattenTokens.js';
import formats from '../lib/common/formats.js';
import { restore, stubMethod } from 'hanbi';

function traverseObj(obj, fn) {
for (let key in obj) {
Expand Down Expand Up @@ -52,6 +53,7 @@ const test_props = {
// extend method is called by StyleDictionary constructor, therefore we're basically testing both things here
describe('StyleDictionary class', () => {
beforeEach(() => {
restore();
clearOutput();
});

Expand Down Expand Up @@ -344,6 +346,126 @@ describe('StyleDictionary class', () => {
});
});

describe('reference errors', () => {
it('should throw an error by default if broken references are encountered', async () => {
const sd = new StyleDictionary({
tokens: {
foo: {
value: '{bar}',
type: 'other',
},
},
platforms: {
css: {},
},
});

await expect(sd.exportPlatform('css')).to.eventually.be.rejectedWith(`
Reference Errors:
Some token references (1) could not be found.
Use log.verbosity "verbose" or use CLI option --verbose for more details.
`);
});

it('should only log an error if broken references are encountered and log.errors.brokenReferences is set to console', async () => {
const stub = stubMethod(console, 'error');
const sd = new StyleDictionary({
log: {
errors: {
brokenReferences: 'console',
},
},
tokens: {
foo: {
value: '{bar}',
type: 'other',
},
},
platforms: {
css: {},
},
});
await sd.exportPlatform('css');
expect(stub.firstCall.args[0]).to.equal(`
Reference Errors:
Some token references (1) could not be found.
Use log.verbosity "verbose" or use CLI option --verbose for more details.
`);
});

it('should allow silencing broken references errors with log.verbosity set to silent and log.errors.brokenReferences set to console', async () => {
const stub = stubMethod(console, 'error');
const sd = new StyleDictionary({
log: {
verbosity: 'silent',
errors: {
brokenReferences: 'console',
},
},
tokens: {
foo: {
value: '{bar}',
type: 'other',
},
},
platforms: {
css: {},
},
});
await sd.exportPlatform('css');
expect(stub.callCount).to.equal(0);
});

it('should resolve correct references when the tokenset contains broken references and log.errors.brokenReferences is set to console', async () => {
const stub = stubMethod(console, 'error');
const sd = new StyleDictionary({
log: {
errors: {
brokenReferences: 'console',
},
},
tokens: {
foo: {
value: '{bar}',
type: 'other',
},
baz: {
value: '8px',
type: 'dimension',
},
qux: {
value: '{baz}',
type: 'dimension',
},
},
platforms: {
css: {},
},
});
const transformed = await sd.exportPlatform('css');
expect(stub.firstCall.args[0]).to.equal(`
Reference Errors:
Some token references (1) could not be found.
Use log.verbosity "verbose" or use CLI option --verbose for more details.
`);

expect(clearSDMeta(transformed)).to.eql({
foo: {
value: '{bar}',
type: 'other',
},
baz: {
value: '8px',
type: 'dimension',
},
qux: {
value: '8px',
type: 'dimension',
},
});
});
});

describe('expand object value tokens', () => {
it('should not expand object value tokens by default', async () => {
const input = {
Expand Down
20 changes: 20 additions & 0 deletions __tests__/__helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import { expect } from 'chai';
import { fs } from 'style-dictionary/fs';
import { resolve } from '../lib/resolve.js';
import isPlainObject from 'is-plain-obj';

export const cleanConsoleOutput = (str) => {
const arr = str
Expand Down Expand Up @@ -80,3 +81,22 @@ export function fixDate() {
return constantDate;
};
}

export function clearSDMeta(tokens) {
const copy = structuredClone(tokens);
function recurse(slice) {
if (isPlainObject(slice)) {
if (Object.hasOwn(slice, 'value')) {
['path', 'original', 'name', 'attributes', 'filePath', 'isSource'].forEach((prop) => {
delete slice[prop];
});
} else {
Object.values(slice).forEach((prop) => {
recurse(prop);
});
}
}
}
recurse(copy);
return copy;
}
8 changes: 5 additions & 3 deletions __tests__/transform/tokenSetup.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,17 @@ import tokenSetup from '../../lib/transform/tokenSetup.js';
describe('transform', () => {
describe('tokenSetup', () => {
it('should error if property is not an object', () => {
expect(tokenSetup.bind(null, null, 'foo', [])).to.throw('Property object must be an object');
expect(tokenSetup.bind(null, null, 'foo', [])).to.throw(
'Token object must be of type "object"',
);
});

it('should error if name in not a string', () => {
expect(tokenSetup.bind(null, {}, null, [])).to.throw('Name must be a string');
expect(tokenSetup.bind(null, {}, null, [])).to.throw('Token name must be a string');
});

it('should error path is not an array', () => {
expect(tokenSetup.bind(null, {}, 'name', null)).to.throw('Path must be an array');
expect(tokenSetup.bind(null, {}, 'name', null)).to.throw('Token path must be an array');
});

it('should work if all the args are proper', () => {
Expand Down
37 changes: 30 additions & 7 deletions __tests__/utils/reference/getReferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
*/

import { expect } from 'chai';
import { _getReferences, getReferences } from '../../../lib/utils/references/getReferences.js';
import { restore, stubMethod } from 'hanbi';
import { getReferences } from '../../../lib/utils/references/getReferences.js';

const tokens = {
color: {
Expand Down Expand Up @@ -50,38 +51,60 @@ describe('utils', () => {
describe('reference', () => {
describe('getReferences()', () => {
describe('public API', () => {
beforeEach(() => {
restore();
});

it('should not collect errors but rather throw immediately when using public API', () => {
expect(() => getReferences('{foo.bar}', tokens)).to.throw(
`tries to reference foo.bar, which is not defined.`,
`Tries to reference foo.bar, which is not defined.`,
);
});

it('should not collect errors but rather throw immediately when using public API', async () => {
const badFn = () => getReferences('{foo.bar}', tokens);
expect(badFn).to.throw(`Tries to reference foo.bar, which is not defined.`);
});

it('should allow warning immediately when references are filtered out', async () => {
const stub = stubMethod(console, 'warn');
const clonedTokens = structuredClone(tokens);
delete clonedTokens.color.red;
getReferences('{color.red}', clonedTokens, {
unfilteredTokens: tokens,
warnImmediately: true,
});
expect(stub.firstCall.args[0]).to.equal(
`Filtered out token references were found: color.red`,
);
});
});

it(`should return an empty array if the value has no references`, () => {
expect(_getReferences(tokens.color.red.value, tokens)).to.eql([]);
expect(getReferences(tokens.color.red.value, tokens)).to.eql([]);
});

it(`should work with a single reference`, () => {
expect(_getReferences(tokens.color.danger.value, tokens)).to.eql([
expect(getReferences(tokens.color.danger.value, tokens)).to.eql([
{ ref: ['color', 'red'], value: '#f00' },
]);
});

it(`should work with object values`, () => {
expect(_getReferences(tokens.border.primary.value, tokens)).to.eql([
expect(getReferences(tokens.border.primary.value, tokens)).to.eql([
{ ref: ['color', 'red'], value: '#f00' },
{ ref: ['size', 'border'], value: '2px' },
]);
});

it(`should work with objects that have numbers`, () => {
expect(_getReferences(tokens.border.secondary.value, tokens)).to.eql([
expect(getReferences(tokens.border.secondary.value, tokens)).to.eql([
{ ref: ['color', 'red'], value: '#f00' },
]);
});

it(`should work with interpolated values`, () => {
expect(_getReferences(tokens.border.tertiary.value, tokens)).to.eql([
expect(getReferences(tokens.border.tertiary.value, tokens)).to.eql([
{ ref: ['size', 'border'], value: '2px' },
{ ref: ['color', 'red'], value: '#f00' },
]);
Expand Down
Loading