feat: improve and test ref utils error handling (#1077)

Author: jorenbroekema

.changeset/kind-rats-invite.md

@@ -0,0 +1,5 @@
+---
+'style-dictionary': minor
+---
+
+Improve and test the error handling of standalone usage of reference utilities.

__integration__/logging/__snapshots__/platform.test.snap.js

@@ -21,7 +21,7 @@ Unknown transformGroup "foo" found in platform "css":
 snapshots["integration logging platform property reference errors should throw and notify users of unknown references"] = 
 `
 Property Reference Errors:
-Reference doesn't exist: color.danger.value tries to reference color.red.value, which is not defined
+Reference doesn't exist: color.danger.value tries to reference color.red.value, which is not defined.
 
 Problems were found when trying to resolve property references`;
 /* end snapshot integration logging platform property reference errors should throw and notify users of unknown references */

__tests__/utils/reference/getReferences.test.js

@@ -12,7 +12,9 @@
  */
 
 import { expect } from 'chai';
-import getReferences from '../../../lib/utils/references/getReferences.js';
+import getReferences, {
+  getReferences as publicGetReferences,
+} from '../../../lib/utils/references/getReferences.js';
 
 const tokens = {
   color: {
@@ -49,6 +51,14 @@ const tokens = {
 describe('utils', () => {
   describe('reference', () => {
     describe('getReferences()', () => {
+      describe('public API', () => {
+        it('should not collect errors but rather throw immediately when using public API', () => {
+          expect(() => publicGetReferences('{foo.bar}', tokens)).to.throw(
+            `Reference doesn't exist: tries to reference foo.bar, which is not defined.`,
+          );
+        });
+      });
+
       it(`should return an empty array if the value has no references`, () => {
         expect(getReferences(tokens.color.red.value, tokens)).to.eql([]);
       });

__tests__/utils/reference/resolveReferences.test.js

@@ -12,7 +12,10 @@
  */
 import { expect } from 'chai';
 import { fileToJSON } from '../../__helpers.js';
-import { resolveReferences } from '../../../lib/utils/references/resolveReferences.js';
+import {
+  _resolveReferences as resolveReferences,
+  resolveReferences as publicResolveReferences,
+} from '../../../lib/utils/references/resolveReferences.js';
 import GroupMessages from '../../../lib/utils/groupMessages.js';
 
 const PROPERTY_REFERENCE_WARNINGS = GroupMessages.GROUP.PropertyReferenceWarnings;
@@ -24,6 +27,21 @@ describe('utils', () => {
         GroupMessages.clear(PROPERTY_REFERENCE_WARNINGS);
       });
 
+      describe('public API', () => {
+        it('should not collect errors but rather throw immediately when using public API', () => {
+          const obj = fileToJSON('__tests__/__json_files/multiple_reference_errors.json');
+          expect(() => publicResolveReferences(obj.a.b, obj)).to.throw(
+            `Reference doesn't exist: tries to reference b.a, which is not defined.`,
+          );
+          expect(() => publicResolveReferences(obj.a.c, obj)).to.throw(
+            `Reference doesn't exist: tries to reference b.c, which is not defined.`,
+          );
+          expect(() => publicResolveReferences(obj.a.d, obj)).to.throw(
+            `Reference doesn't exist: tries to reference d, which is not defined.`,
+          );
+        });
+      });
+
       it('should do simple references', () => {
         const test = resolveReferences('{foo}', fileToJSON('__tests__/__json_files/simple.json'));
         expect(test).to.equal('bar');
@@ -105,8 +123,8 @@ describe('utils', () => {
         expect(resolveReferences(obj.foo, obj)).to.be.undefined;
         expect(resolveReferences(obj.error, obj)).to.be.undefined;
         expect(GroupMessages.fetchMessages(PROPERTY_REFERENCE_WARNINGS)).to.eql([
-          "Reference doesn't exist: tries to reference bar, which is not defined",
-          "Reference doesn't exist: tries to reference a.b.d, which is not defined",
+          "Reference doesn't exist: tries to reference bar, which is not defined.",
+          "Reference doesn't exist: tries to reference a.b.d, which is not defined.",
         ]);
       });
 
@@ -202,9 +220,9 @@ describe('utils', () => {
         expect(GroupMessages.count(PROPERTY_REFERENCE_WARNINGS)).to.equal(3);
         expect(JSON.stringify(GroupMessages.fetchMessages(PROPERTY_REFERENCE_WARNINGS))).to.equal(
           JSON.stringify([
-            "Reference doesn't exist: tries to reference b.a, which is not defined",
-            "Reference doesn't exist: tries to reference b.c, which is not defined",
-            "Reference doesn't exist: tries to reference d, which is not defined",
+            "Reference doesn't exist: tries to reference b.a, which is not defined.",
+            "Reference doesn't exist: tries to reference b.c, which is not defined.",
+            "Reference doesn't exist: tries to reference d, which is not defined.",
           ]),
         );
       });

__tests__/utils/resolveObject.test.js

@@ -321,9 +321,9 @@ describe('utils', () => {
       expect(GroupMessages.count(PROPERTY_REFERENCE_WARNINGS)).to.equal(3);
       expect(JSON.stringify(GroupMessages.fetchMessages(PROPERTY_REFERENCE_WARNINGS))).to.equal(
         JSON.stringify([
-          "Reference doesn't exist: a.b tries to reference b.a, which is not defined",
-          "Reference doesn't exist: a.c tries to reference b.c, which is not defined",
-          "Reference doesn't exist: a.d tries to reference d, which is not defined",
+          "Reference doesn't exist: a.b tries to reference b.a, which is not defined.",
+          "Reference doesn't exist: a.c tries to reference b.c, which is not defined.",
+          "Reference doesn't exist: a.d tries to reference d, which is not defined.",
         ]),
       );
     });

lib/buildFile.js

@@ -110,7 +110,7 @@ export default function buildFile(file, platform = {}, dictionary) {
     }
   });
 
-  let tokenNamesCollisionCount = GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS);
+  const tokenNamesCollisionCount = GroupMessages.count(PROPERTY_NAME_COLLISION_WARNINGS);
   fs.writeFileSync(
     fullDestination,
     format(
@@ -122,7 +122,7 @@ export default function buildFile(file, platform = {}, dictionary) {
     ),
   );
 
-  let filteredReferencesCount = GroupMessages.count(GroupMessages.GROUP.FilteredOutputReferences);
+  const filteredReferencesCount = GroupMessages.count(GroupMessages.GROUP.FilteredOutputReferences);
 
   // don't show name collision warnings for nested type formats
   // because they are not relevant.
@@ -131,13 +131,13 @@ export default function buildFile(file, platform = {}, dictionary) {
   } else {
     const warnHeader = `⚠️ ${fullDestination}`;
     if (tokenNamesCollisionCount > 0) {
-      let tokenNamesCollisionWarnings = GroupMessages.fetchMessages(
+      const tokenNamesCollisionWarnings = GroupMessages.fetchMessages(
         PROPERTY_NAME_COLLISION_WARNINGS,
       ).join('\n    ');
-      let title = `While building ${chalk
+      const title = `While building ${chalk
         .rgb(255, 69, 0)
         .bold(destination)}, token collisions were found; output may be unexpected.`;
-      let help = chalk.rgb(
+      const help = chalk.rgb(
         255,
         165,
         0,
@@ -149,7 +149,7 @@ export default function buildFile(file, platform = {}, dictionary) {
           '* overly inclusive file filters',
         ].join('\n    '),
       );
-      let warn = `${warnHeader}\n${title}\n    ${tokenNamesCollisionWarnings}\n${help}`;
+      const warn = `${warnHeader}\n${title}\n    ${tokenNamesCollisionWarnings}\n${help}`;
       if (platform?.log === 'error') {
         throw new Error(warn);
       } else {
@@ -158,20 +158,20 @@ export default function buildFile(file, platform = {}, dictionary) {
     }
 
     if (filteredReferencesCount > 0) {
-      let filteredReferencesWarnings = GroupMessages.flush(
+      const filteredReferencesWarnings = GroupMessages.flush(
         GroupMessages.GROUP.FilteredOutputReferences,
       ).join('\n    ');
-      let title = `While building ${chalk
+      const title = `While building ${chalk
         .rgb(255, 69, 0)
         .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.rgb(
+      const help = chalk.rgb(
         255,
         165,
         0,
       )(['This is caused when combining a filter and `outputReferences`.'].join('\n    '));
-      let warn = `${warnHeader}\n${title}\n    ${filteredReferencesWarnings}\n${help}`;
+      const warn = `${warnHeader}\n${title}\n    ${filteredReferencesWarnings}\n${help}`;
       if (platform?.log === 'error') {
         throw new Error(warn);
       } else {

lib/common/formatHelpers/createPropertyFormatter.js

@@ -10,8 +10,8 @@
  * CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
  * and limitations under the License.
  */
-
-import { usesReferences, getReferences } from '../../utils/index.js';
+import getReferences from '../../utils/references/getReferences.js';
+import usesReferences from '../../utils/references/usesReferences.js';
 
 /**
  * @typedef {import('../../../types/DesignToken.d.ts').Dictionary} Dictionary

lib/common/formatHelpers/sortByReference.js

@@ -11,7 +11,8 @@
  * and limitations under the License.
  */
 
-import { usesReferences, getReferences } from 'style-dictionary/utils';
+import usesReferences from '../../utils/references/usesReferences.js';
+import getReferences from '../../utils/references/getReferences.js';
 
 /**
  * @typedef {import('../../../types/DesignToken.d.ts').TransformedTokens} Tokens

lib/utils/index.js

@@ -12,7 +12,7 @@
  */
 
 import usesReferences from './references/usesReferences.js';
-import getReferences from './references/getReferences.js';
+import { getReferences } from './references/getReferences.js';
 import { resolveReferences } from './references/resolveReferences.js';
 
 // Public style-dictionary/utils API

lib/utils/references/getReferences.js

@@ -10,13 +10,33 @@
  * CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
  * and limitations under the License.
  */
-
 import getPathFromName from './getPathFromName.js';
 import createReferenceRegex from './createReferenceRegex.js';
 import getValueByPath from './getValueByPath.js';
 import GroupMessages from '../groupMessages.js';
 import defaults from './defaults.js';
 
+const FILTER_WARNINGS = GroupMessages.GROUP.FilteredOutputReferences;
+
+/**
+ * @typedef {import('../../../types/Config.d.ts').RegexOptions} RegexOptions
+ * @typedef {import('../../StyleDictionary.js').default} Dictionary
+ * @typedef {import('../../../types/DesignToken.d.ts').TransformedTokens} Tokens
+ * @typedef {import('../../../types/DesignToken.d.ts').TransformedToken} Token
+ *
+ * Public API wrapper around the function below this one
+ *
+ * @memberof Dictionary
+ * @param {string|Object<string, string|any>} value the value that contains a reference
+ * @param {Tokens} tokens the dictionary to search in
+ * @param {RegexOptions & { unfilteredTokens?: Tokens }} [opts]
+ * @param {Token[]} [references] array of token's references because a token's value can contain multiple references due to string interpolation
+ * @returns {Token[]}
+ */
+export function getReferences(value, tokens, opts = {}, references = []) {
+  return _getReferences(value, tokens, opts, references, true);
+}
+
 /**
  * This is a helper function that is added to the dictionary object that
  * is passed to formats and actions. It will resolve a reference giving
@@ -26,19 +46,21 @@ import defaults from './defaults.js';
  * ```css
  * --color-background-base: var(--color-core-white);
  * ```
- * @typedef {import('../../../types/Config.d.ts').RegexOptions} RegexOptions
- * @typedef {import('../../StyleDictionary.js').default} Dictionary
- * @typedef {import('../../../types/DesignToken.d.ts').TransformedTokens} Tokens
- * @typedef {import('../../../types/DesignToken.d.ts').TransformedToken} Token
  *
- * @memberof Dictionary
  * @param {string|Object<string, string|any>} value the value that contains a reference
  * @param {Tokens} tokens the dictionary to search in
  * @param {RegexOptions & { unfilteredTokens?: Tokens }} [opts]
  * @param {Token[]} [references] array of token's references because a token's value can contain multiple references due to string interpolation
+ * @param {boolean} [throwImmediately]
  * @returns {Token[]}
  */
-export default function getReferences(value, tokens, opts = {}, references = []) {
+export default function _getReferences(
+  value,
+  tokens,
+  opts = {},
+  references = [],
+  throwImmediately = false,
+) {
   const regex = createReferenceRegex(opts);
 
   /**
@@ -55,14 +77,21 @@ export default function getReferences(value, tokens, opts = {}, references = [])
 
     let ref = getValueByPath(pathName, tokens);
 
-    if (!ref && opts.unfilteredTokens) {
+    if (ref === undefined && opts.unfilteredTokens) {
+      if (!throwImmediately) {
+        // warn the user about this
+        GroupMessages.add(FILTER_WARNINGS, variable);
+      }
       // fall back on unfilteredTokens as it is unfiltered
       ref = getValueByPath(pathName, opts.unfilteredTokens);
-      // and warn the user about this
-      GroupMessages.add(GroupMessages.GROUP.FilteredOutputReferences, variable);
     }
+
     if (ref !== undefined) {
       references.push(ref);
+    } else if (throwImmediately) {
+      throw new Error(
+        `Reference doesn't exist: tries to reference ${variable}, which is not defined.`,
+      );
     }
     return '';
   }
@@ -83,7 +112,7 @@ export default function getReferences(value, tokens, opts = {}, references = [])
       }
       // if it is an object, we go further down the rabbit hole
       if (value.hasOwnProperty(key) && typeof value[key] === 'object') {
-        getReferences(value[key], tokens, opts, references);
+        _getReferences(value[key], tokens, opts, references);
       }
     }
   }

lib/utils/references/resolveReferences.js

@@ -28,14 +28,15 @@ const PROPERTY_REFERENCE_WARNINGS = GroupMessages.GROUP.PropertyReferenceWarning
  * @typedef {import('../../../types/DesignToken.d.ts').DesignToken} DesignToken
  */
 
-/** Utility to resolve references inside a string value
+/**
+ * Public API wrapper around the functon below this one
  * @param {string} value
  * @param {DesignTokens} tokens
  * @param {RefOpts} [opts]
  * @returns {string|number|undefined}
  */
 export function resolveReferences(value, tokens, opts) {
-  return _resolveReferences(value, tokens, opts);
+  return _resolveReferences(value, tokens, { ...opts, throwImmediately: true });
 }
 
 /**
@@ -59,6 +60,7 @@ export function _resolveReferences(
     stack = [],
     foundCirc = {},
     firstIteration = true,
+    throwImmediately = false,
   } = {},
 ) {
   const reg = regex ?? createReferenceRegex({ opening_character, closing_character, separator });
@@ -136,10 +138,15 @@ export function _resolveReferences(
             circStack.push(reference);
 
             // Add circ reference info to our list of warning messages
-            GroupMessages.add(
-              PROPERTY_REFERENCE_WARNINGS,
-              'Circular definition cycle:  ' + circStack.join(', '),
-            );
+            const warning = `Circular definition cycle: ${circStack.join(', ')}`;
+            if (throwImmediately) {
+              throw new Error(warning);
+            } else {
+              GroupMessages.add(
+                PROPERTY_REFERENCE_WARNINGS,
+                'Circular definition cycle:  ' + circStack.join(', '),
+              );
+            }
           } else {
             to_ret = _resolveReferences(to_ret, tokens, {
               regex: reg,
@@ -164,12 +171,14 @@ export function _resolveReferences(
       // User might have passed current_context option which is path (arr) pointing to key
       // that this value is associated with, helpful for debugging
       const context = getName(current_context, { separator });
-      GroupMessages.add(
-        PROPERTY_REFERENCE_WARNINGS,
-        `Reference doesn't exist:${
-          context ? ` ${context}` : ''
-        } tries to reference ${variable}, which is not defined`,
-      );
+      const warning = `Reference doesn't exist:${
+        context ? ` ${context}` : ''
+      } tries to reference ${variable}, which is not defined.`;
+      if (throwImmediately) {
+        throw new Error(warning);
+      } else {
+        GroupMessages.add(PROPERTY_REFERENCE_WARNINGS, warning);
+      }
       to_ret = ref;
     }
     stack.pop();

package-lock.json

@@ -43,6 +43,7 @@ export interface ResolveReferenceOptionsInternal extends ResolveReferenceOptions
   stack?: string[];
   foundCirc?: Record<string, boolean>;
   firstIteration?: boolean;
+  throwImmediately?: boolean;
 }
 
 export interface PlatformConfig extends RegexOptions {