Skip to content

Commit

Permalink
feat: allow not throwing on broken refs
Browse files Browse the repository at this point in the history
  • Loading branch information
jorenbroekema committed Jun 14, 2024
1 parent 1b8bdff commit c172230
Show file tree
Hide file tree
Showing 19 changed files with 249 additions and 70 deletions.
21 changes: 21 additions & 0 deletions .changeset/chilly-numbers-work.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
'style-dictionary': minor
---

Allow not throwing fatal errors on broken token references/aliases, but `console.error` instead.
`resolveReferences` and `getReferences` both allow passing `throwOnBrokenReferences` option, setting this to false will prevent a fatal error.
You can also configure this on global/platform `log` property:

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

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

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
50 changes: 50 additions & 0 deletions __tests__/StyleDictionary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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,54 @@ 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.
`);
});
});

describe('expand object value tokens', () => {
it('should not expand object value tokens by default', async () => {
const input = {
Expand Down
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
25 changes: 24 additions & 1 deletion __tests__/utils/reference/getReferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/

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

const tokens = {
Expand Down Expand Up @@ -50,9 +51,31 @@ 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', () => {
const stub = stubMethod(console, 'error');
getReferences('{foo.bar}', tokens, { throwOnBrokenReferences: false });
expect(stub.firstCall.args[0]).to.equal(
`Tries to reference foo.bar, which is not defined.`,
);
});

it('should warn 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 });
expect(stub.firstCall.args[0]).to.equal(
`Filtered out token references were found: color.red`,
);
});
});
Expand Down
28 changes: 28 additions & 0 deletions __tests__/utils/reference/resolveReferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* and limitations under the License.
*/
import { expect } from 'chai';
import { restore, stubMethod } from 'hanbi';
import { fileToJSON } from '../../__helpers.js';
import {
_resolveReferences as resolveReferences,
Expand All @@ -25,6 +26,7 @@ describe('utils', () => {
describe('resolveReferences', () => {
beforeEach(() => {
GroupMessages.clear(PROPERTY_REFERENCE_WARNINGS);
restore();
});

describe('public API', () => {
Expand All @@ -40,6 +42,23 @@ describe('utils', () => {
`tries to reference d, which is not defined.`,
);
});

it('should only console.error if throwOnBrokenReferences is disabled', async () => {
const stub = stubMethod(console, 'error');
publicResolveReferences('{foo.bar}', {}, { throwOnBrokenReferences: false });
expect(stub.firstCall.args[0]).to.equal(
`tries to reference foo.bar, which is not defined.`,
);
});

it('should gracefully handle basic circular references if throwOnBrokenReferences is disabled', () => {
const stub = stubMethod(console, 'error');
const obj = fileToJSON('__tests__/__json_files/circular.json');
expect(publicResolveReferences(obj.a, obj, { throwOnBrokenReferences: false })).to.equal(
'{b}',
);
expect(stub.firstCall.args[0]).to.equal('Circular definition cycle: b, c, d, a, b');
});
});

it('should do simple references', () => {
Expand Down Expand Up @@ -137,6 +156,15 @@ describe('utils', () => {
);
});

it('should gracefully handle basic circular references', () => {
const obj = fileToJSON('__tests__/__json_files/circular.json');
expect(resolveReferences(obj.a, obj)).to.equal('{b}');
expect(GroupMessages.count(PROPERTY_REFERENCE_WARNINGS)).to.equal(1);
expect(JSON.stringify(GroupMessages.fetchMessages(PROPERTY_REFERENCE_WARNINGS))).to.equal(
JSON.stringify(['Circular definition cycle: b, c, d, a, b']),
);
});

it('should gracefully handle basic and nested circular references', () => {
const obj = fileToJSON('__tests__/__json_files/circular_2.json');
expect(resolveReferences(obj.j, obj)).to.equal('{a.b.c}');
Expand Down
16 changes: 8 additions & 8 deletions docs/src/content/docs/reference/Utils/references.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ resolveReferences('solid {spacing.2} {colors.black}', sd.tokens); // alternative
resolveReferences('solid {spacing.2} {colors.black}', sd.tokens, { usesDtcg: true }); // Assumes DTCG spec format, with $ prefix ($value, $type)
```

:::note
You can pass a third `options` argument where you can pass some configuration options for how references are resolved
Most notable option for public usage is `usesDtcg`, if set to true, the `resolveReferences` utility will assume DTCG syntax (`$value` props).
:::
You can pass a third `options` argument where you can pass some configuration options for how references are resolved:

- `usesDtcg` boolean, if set to true, the `resolveReferences` utility will assume DTCG syntax (`$value` props)
- `throwOnBrokenReferences` boolean, if set to true, it will `console.error` for reference errors instead of fatally throw

## getReferences

Expand Down Expand Up @@ -97,10 +97,10 @@ getReferences('solid {spacing.2} {colors.black}', sd.tokens, { usesDtcg: true })
*/
```

:::note
You can pass a third `options` argument where you can pass some configuration options for how references are resolved
Most notable option for public usage is `usesDtcg`, if set to true, the `resolveReferences` utility will assume DTCG syntax (`$value` props)
:::
You can pass a third `options` argument where you can pass some configuration options for how references are resolved:

- `usesDtcg` boolean, if set to true, the `resolveReferences` utility will assume DTCG syntax (`$value` props)
- `throwOnBrokenReferences` boolean, if set to true, it will `console.error` for reference errors instead of fatally throw

### Complicated example

Expand Down
15 changes: 10 additions & 5 deletions docs/src/content/docs/reference/logging.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,22 @@ const sd = new StyleDictionary({
log: {
warnings: 'warn', // 'warn' | 'error' | 'disabled'
verbosity: 'default', // 'default' | 'silent' | 'verbose'
errors: {
brokenReferences: 'throw', // 'throw' | 'console'
},
},
});
```

> `log` can also be set on platform specific configuration
| Param | Type | Description |
| --------------- | ----------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `log` | `Object` | |
| `log.warnings` | `'warn' \| 'error' \| 'disabled'` | Whether warnings should be logged as warnings, thrown as errors or disabled entirely. Defaults to 'warn' |
| `log.verbosity` | `'default' \|'silent' \| 'verbose'` | How verbose logs should be, default value is 'default'. 'silent' means no logs at all apart from fatal errors. 'verbose' means detailed error messages for debugging |
| Param | Type | Description |
| ----------------------------- | ----------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `log` | `Object` | |
| `log.warnings` | `'warn' \| 'error' \| 'disabled'` | Whether warnings should be logged as warnings, thrown as errors or disabled entirely. Defaults to 'warn' |
| `log.verbosity` | `'default' \|'silent' \| 'verbose'` | How verbose logs should be, default value is 'default'. 'silent' means no logs at all apart from fatal errors. 'verbose' means detailed error messages for debugging |
| `log.errors` | `Object` | How verbose logs should be, default value is 'default'. 'silent' means no logs at all apart from fatal errors. 'verbose' means detailed error messages for debugging |
| `log.errors.brokenReferences` | `'throw' \| 'console'` | Whether broken references in tokens should throw a fatal error or only a `console.error` without exiting the process. |

There are five types of warnings that will be thrown as errors instead of being logged as warnings when `log.warnings` is set to `error`:

Expand Down
4 changes: 2 additions & 2 deletions docs/starlight-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ export default {
'Export your Design Tokens to any platform. iOS, Android, CSS, JS, HTML, sketch files, style documentation, or anything you can think of. Forward-compatible with Design Token Community Group spec.',
logo: { src: './src/assets/logo.png', alt: 'Style-Dictionary logo, Pascal the chameleon.' },
editLink: {
baseUrl: 'https://github.com/amzn/style-dictionary/edit/v4/src/content/docs/',
baseUrl: 'https://github.com/amzn/style-dictionary/edit/v4/docs/src/content/docs/',
},
favicon: '/favicon.png',
social: {
github: 'https://github.com/amzn/style-dictionary',
github: 'https://github.com/amzn/style-dictionary/tree/v4',
slack:
'https://join.slack.com/t/tokens-studio/shared_invite/zt-1p8ea3m6t-C163oJcN9g3~YZTKRgo2hg',
},
Expand Down
18 changes: 11 additions & 7 deletions lib/StyleDictionary.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import cleanActions from './cleanActions.js';
const PROPERTY_VALUE_COLLISIONS = GroupMessages.GROUP.PropertyValueCollisions;
const PROPERTY_REFERENCE_WARNINGS = GroupMessages.GROUP.PropertyReferenceWarnings;
const UNKNOWN_CSS_FONT_PROPS_WARNINGS = GroupMessages.GROUP.UnknownCSSFontProperties;
const FILTER_WARNINGS = GroupMessages.GROUP.FilteredOutputReferences;

/**
* Style Dictionary module
Expand Down Expand Up @@ -118,6 +119,9 @@ export default class StyleDictionary extends Register {
this.log = {
warnings: 'warn',
verbosity: 'default',
errors: {
brokenReferences: 'throw',
},
};
/** @type {string[]} */
this.source = [];
Expand Down Expand Up @@ -503,7 +507,11 @@ export default class StyleDictionary extends Register {
err += `${verbosityInfo}\n`;
}

throw new Error(err);
if (this.log.errors?.brokenReferences === 'throw') {
throw new Error(err);
} else {
console.error(err);
}
}

const unknownPropsWarningCount = GroupMessages.count(UNKNOWN_CSS_FONT_PROPS_WARNINGS);
Expand Down Expand Up @@ -669,9 +677,7 @@ export default class StyleDictionary extends Register {
}),
);

const filteredReferencesCount = GroupMessages.count(
GroupMessages.GROUP.FilteredOutputReferences,
);
const filteredReferencesCount = GroupMessages.count(FILTER_WARNINGS);

// don't show name collision warnings for nested type formats
// because they are not relevant.
Expand Down Expand Up @@ -716,9 +722,7 @@ export default class StyleDictionary extends Register {
}

if (filteredReferencesCount > 0) {
const filteredReferencesWarnings = GroupMessages.flush(
GroupMessages.GROUP.FilteredOutputReferences,
).join('\n ');
const filteredReferencesWarnings = GroupMessages.flush(FILTER_WARNINGS).join('\n ');
const title = `While building ${chalk
.rgb(255, 69, 0)
.bold(
Expand Down
2 changes: 1 addition & 1 deletion lib/cleanFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default async function cleanFiles(platform, vol) {
if (file.format) {
return cleanFile(file, platform, vol);
} else {
throw new Error('Please supply a template or format');
throw new Error('Please supply a format');
}
}),
);
Expand Down
Loading

0 comments on commit c172230

Please sign in to comment.