From 74780038b09f3aa3ae0890bbbb25aebb12532a1c Mon Sep 17 00:00:00 2001 From: jorenbroekema Date: Fri, 14 Jun 2024 12:57:35 +0200 Subject: [PATCH] fix: revert throwOnBrokenReferences, respect silent verbosity --- .changeset/chilly-numbers-work.md | 4 +++- __tests__/StyleDictionary.test.js | 23 +++++++++++++++++++ .../utils/reference/getReferences.test.js | 9 +++----- .../utils/reference/resolveReferences.test.js | 19 +-------------- .../docs/reference/Utils/references.md | 2 -- lib/StyleDictionary.js | 2 +- lib/utils/references/getReferences.js | 12 ++-------- lib/utils/references/resolveReferences.js | 14 ++--------- types/Config.d.ts | 2 -- 9 files changed, 35 insertions(+), 52 deletions(-) diff --git a/.changeset/chilly-numbers-work.md b/.changeset/chilly-numbers-work.md index b04733bba..52907ccb0 100644 --- a/.changeset/chilly-numbers-work.md +++ b/.changeset/chilly-numbers-work.md @@ -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 @@ -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. diff --git a/__tests__/StyleDictionary.test.js b/__tests__/StyleDictionary.test.js index e959e61ac..558763d44 100644 --- a/__tests__/StyleDictionary.test.js +++ b/__tests__/StyleDictionary.test.js @@ -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({ diff --git a/__tests__/utils/reference/getReferences.test.js b/__tests__/utils/reference/getReferences.test.js index 6750719f6..9b254e9e2 100644 --- a/__tests__/utils/reference/getReferences.test.js +++ b/__tests__/utils/reference/getReferences.test.js @@ -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 () => { diff --git a/__tests__/utils/reference/resolveReferences.test.js b/__tests__/utils/reference/resolveReferences.test.js index 6105a03f4..819af33aa 100644 --- a/__tests__/utils/reference/resolveReferences.test.js +++ b/__tests__/utils/reference/resolveReferences.test.js @@ -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, @@ -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', () => { diff --git a/docs/src/content/docs/reference/Utils/references.md b/docs/src/content/docs/reference/Utils/references.md index ea4b4dae0..5f2100cd2 100644 --- a/docs/src/content/docs/reference/Utils/references.md +++ b/docs/src/content/docs/reference/Utils/references.md @@ -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 @@ -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. diff --git a/lib/StyleDictionary.js b/lib/StyleDictionary.js index 367c56914..8ce40ac7b 100644 --- a/lib/StyleDictionary.js +++ b/lib/StyleDictionary.js @@ -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); } } diff --git a/lib/utils/references/getReferences.js b/lib/utils/references/getReferences.js index 564591331..99a6af1df 100644 --- a/lib/utils/references/getReferences.js +++ b/lib/utils/references/getReferences.js @@ -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); /** @@ -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 ''; diff --git a/lib/utils/references/resolveReferences.js b/lib/utils/references/resolveReferences.js index e64795cd2..856e39751 100644 --- a/lib/utils/references/resolveReferences.js +++ b/lib/utils/references/resolveReferences.js @@ -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 = [], @@ -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, @@ -160,7 +155,6 @@ export function _resolveReferences( regex: reg, ignorePaths, usesDtcg, - throwOnBrokenReferences, warnImmediately, current_context, separator, @@ -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); } diff --git a/types/Config.d.ts b/types/Config.d.ts index 9e98d1bcd..bb78379ee 100644 --- a/types/Config.d.ts +++ b/types/Config.d.ts @@ -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; }