Skip to content

Commit

Permalink
fix: revert throwOnBrokenReferences, respect silent verbosity
Browse files Browse the repository at this point in the history
  • Loading branch information
jorenbroekema committed Jun 15, 2024
1 parent 09a3b72 commit b13e7cb
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 56 deletions.
4 changes: 3 additions & 1 deletion .changeset/chilly-numbers-work.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
---

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
Expand All @@ -18,4 +18,6 @@ You can also configure this on global/platform `log` property:

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.
23 changes: 23 additions & 0 deletions __tests__/StyleDictionary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,29 @@ 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({
Expand Down
7 changes: 3 additions & 4 deletions __tests__/__helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,9 @@ export function clearSDMeta(tokens) {
function recurse(slice) {
if (isPlainObject(slice)) {
if (Object.hasOwn(slice, 'value')) {
delete slice.path;
delete slice.original;
delete slice.name;
delete slice.attributes;
['path', 'original', 'name', 'attributes', 'filePath', 'isSource'].forEach((prop) => {
delete slice[prop];
});
} else {
Object.values(slice).forEach((prop) => {
recurse(prop);
Expand Down
9 changes: 3 additions & 6 deletions __tests__/utils/reference/getReferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,9 @@ describe('utils', () => {
);
});

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 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 () => {
Expand Down
19 changes: 1 addition & 18 deletions __tests__/utils/reference/resolveReferences.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* and limitations under the License.
*/
import { expect } from 'chai';
import { restore, stubMethod } from 'hanbi';
import { restore } from 'hanbi';
import { fileToJSON } from '../../__helpers.js';
import {
_resolveReferences as resolveReferences,
Expand Down Expand Up @@ -42,23 +42,6 @@ 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
2 changes: 0 additions & 2 deletions docs/src/content/docs/reference/Utils/references.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ resolveReferences('solid {spacing.2} {colors.black}', sd.tokens, { usesDtcg: tru
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
- `warnImmediately` boolean, `true` by default. You should only set this to `false` if you know that this utility is used used inside of the Transform lifecycle hook of Style Dictionary, allowing the errors to be grouped and only thrown at the end of the transform step (end of [exportPlatform](/reference/api#exportplatform) method).

## getReferences
Expand Down Expand Up @@ -101,7 +100,6 @@ getReferences('solid {spacing.2} {colors.black}', sd.tokens, { usesDtcg: true })
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
- `unfilteredTokens`, assuming the second `tokens` argument is your filtered `tokens` object where [filters](/reference/hooks/filters) have already done its work, you'll likely want to pass the unfiltered set in case the reference you're trying to find no longer exist in the filtered set, but you still want to get the reference values. This is useful when you're writing your own custom format with an `outputReferences` feature and you want to prevent outputting refs that no longer exist in the filtered set.
- `warnImmediately` boolean, `true` by default. You should only set this to `false` if you know that this utility is used inside of the Format lifecycle hook of Style Dictionary, allowing the errors to be grouped and only thrown at the end of the format step.

Expand Down
2 changes: 1 addition & 1 deletion lib/StyleDictionary.js
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ export default class StyleDictionary extends Register {

if (this.log.errors?.brokenReferences === 'throw') {
throw new Error(err);
} else {
} else if (this.log.verbosity !== 'silent') {
console.error(err);
}
}
Expand Down
12 changes: 2 additions & 10 deletions lib/utils/references/getReferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,7 @@ const FILTER_WARNINGS = GroupMessages.GROUP.FilteredOutputReferences;
* @returns {Token[]}
*/
export function getReferences(value, tokens, opts = {}, references = []) {
const {
usesDtcg,
separator,
throwOnBrokenReferences = true,
warnImmediately = true,
unfilteredTokens,
} = opts;
const { usesDtcg, separator, warnImmediately = true, unfilteredTokens } = opts;
const regex = createReferenceRegex(opts);

/**
Expand Down Expand Up @@ -87,10 +81,8 @@ export function getReferences(value, tokens, opts = {}, references = []) {
}
} else {
const errMessage = `Tries to reference ${variable}, which is not defined.`;
if (throwOnBrokenReferences) {
if (warnImmediately) {
throw new Error(errMessage);
} else {
console.error(errMessage);
}
}
return '';
Expand Down
14 changes: 2 additions & 12 deletions lib/utils/references/resolveReferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export function _resolveReferences(
opening_character = defaults.opening_character,
closing_character = defaults.closing_character,
usesDtcg = false,
throwOnBrokenReferences = true,
warnImmediately = true,
// for internal usage
ignorePaths = [],
Expand Down Expand Up @@ -144,11 +143,7 @@ export function _resolveReferences(
// Add circ reference info to our list of warning messages
const warning = `Circular definition cycle: ${circStack.join(', ')}`;
if (warnImmediately) {
if (throwOnBrokenReferences) {
throw new Error(warning);
} else {
console.error(warning);
}
throw new Error(warning);
} else {
GroupMessages.add(
PROPERTY_REFERENCE_WARNINGS,
Expand All @@ -160,7 +155,6 @@ export function _resolveReferences(
regex: reg,
ignorePaths,
usesDtcg,
throwOnBrokenReferences,
warnImmediately,
current_context,
separator,
Expand All @@ -186,11 +180,7 @@ export function _resolveReferences(
context ? `${context} ` : ''
}tries to reference ${variable}, which is not defined.`;
if (warnImmediately) {
if (throwOnBrokenReferences) {
throw new Error(warning);
} else {
console.error(warning);
}
throw new Error(warning);
} else {
GroupMessages.add(PROPERTY_REFERENCE_WARNINGS, warning);
}
Expand Down
2 changes: 0 additions & 2 deletions types/Config.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,12 @@ export interface RegexOptions {

export interface GetReferencesOptions extends RegexOptions {
usesDtcg?: boolean;
throwOnBrokenReferences?: boolean;
unfilteredTokens?: DesignTokens;
warnImmediately?: boolean;
}

export interface ResolveReferencesOptions extends RegexOptions {
usesDtcg?: boolean;
throwOnBrokenReferences?: boolean;
warnImmediately?: boolean;
}

Expand Down

0 comments on commit b13e7cb

Please sign in to comment.