fix: logging to be in sequence per platform (#1161)

amzn · Apr 16, 2024 · 6cfce97 · 6cfce97
1 parent 51efc17
commit 6cfce97
Show file tree

Hide file tree

Showing 8 changed files with 68 additions and 58 deletions.
diff --git a/.changeset/hip-candles-flow.md b/.changeset/hip-candles-flow.md
@@ -0,0 +1,5 @@
+---
+'style-dictionary': patch
+---
+
+Fix logging to be ordered by platform when building or cleaning platforms. This now happens in parallel, resulting in the logs being ordered randomly which was a small regression to the logging experience.
diff --git a/__integration__/logging/file.test.js b/__integration__/logging/file.test.js
@@ -248,9 +248,6 @@ describe(`integration`, () => {
             error = e;
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-          // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
 
         it(`should throw a brief error of name collisions with log level set to error on platform level`, async () => {
@@ -280,8 +277,6 @@ describe(`integration`, () => {
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
           // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
 
         it(`should warn user of name collisions with a detailed message through "verbose" verbosity`, async () => {
@@ -335,9 +330,6 @@ describe(`integration`, () => {
             error = e;
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-          // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
       });
 
@@ -456,8 +448,6 @@ describe(`integration`, () => {
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
           // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
 
         it(`should throw a brief error of filtered references with log level set to error on platform level`, async () => {
@@ -491,8 +481,6 @@ describe(`integration`, () => {
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
           // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
 
         it(`should warn user of filtered references with a detailed message through "verbose" verbosity`, async () => {
@@ -554,9 +542,6 @@ describe(`integration`, () => {
             error = e;
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-          // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
       });
     });

diff --git a/__integration__/logging/platform.test.js b/__integration__/logging/platform.test.js
@@ -54,9 +54,6 @@ describe(`integration`, () => {
           error = e;
         }
         await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-        // only log is the platform name at the start of the buildPlatform method
-        expect(stub.callCount).to.equal(1);
-        expect(stub.firstCall.args).to.eql(['\ncss']);
       });
 
       it(`should throw and notify users of unknown transforms`, async () => {
@@ -75,9 +72,6 @@ describe(`integration`, () => {
           error = e;
         }
         await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-        // only log is the platform name at the start of the buildPlatform method
-        expect(stub.callCount).to.equal(1);
-        expect(stub.firstCall.args).to.eql(['\ncss']);
       });
 
       it(`should throw and notify users of unknown transformGroups`, async () => {
@@ -96,9 +90,6 @@ describe(`integration`, () => {
           error = e;
         }
         await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-        // only log is the platform name at the start of the buildPlatform method
-        expect(stub.callCount).to.equal(1);
-        expect(stub.firstCall.args).to.eql(['\ncss']);
       });
 
       describe(`property reference errors`, () => {
@@ -121,9 +112,6 @@ describe(`integration`, () => {
             error = e;
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-          // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
 
         it(`circular references should throw and notify users`, async () => {
@@ -149,9 +137,6 @@ describe(`integration`, () => {
             error = e;
           }
           await expect(cleanConsoleOutput(error.message)).to.matchSnapshot();
-          // only log is the platform name at the start of the buildPlatform method
-          expect(stub.callCount).to.equal(1);
-          expect(stub.firstCall.args).to.eql(['\ncss']);
         });
       });
     });

diff --git a/__tests__/buildFile.test.js b/__tests__/buildFile.test.js
@@ -111,9 +111,8 @@ describe('buildFile', () => {
     });
 
     it('should not warn users if the format is a nested format', async () => {
-      const consoleStub = stubMethod(console, 'log');
-      await buildFile({ destination, format: nestedFormat }, {}, dictionary, {});
-      expect(consoleStub.calledWith(chalk.bold.green(`✔︎ ${destination}`))).to.be.true;
+      const logs = await buildFile({ destination, format: nestedFormat }, {}, dictionary, {});
+      expect(logs.success[0]).to.equal(chalk.bold.green(`✔︎ ${destination}`));
     });
   });
 

diff --git a/lib/StyleDictionary.js b/lib/StyleDictionary.js
@@ -460,11 +460,6 @@ export default class StyleDictionary extends Register {
    */
   async getPlatform(platform) {
     await this.hasInitialized;
-    if (this.log?.verbosity !== 'silent') {
-      // eslint-disable-next-line no-console
-      console.log('\n' + platform);
-    }
-
     if (!this.platforms?.[platform]) {
       throw new Error(`Platform "${platform}" does not exist`);
     }
@@ -490,7 +485,24 @@ export default class StyleDictionary extends Register {
    */
   async buildPlatform(platform) {
     const { dictionary, platformConfig } = await this.getPlatform(platform);
-    await buildFiles(dictionary, platformConfig, this.options, this.volume);
+    // collect logs, buildFiles happens in parallel but we want to log in sequence
+    const logs = await buildFiles(dictionary, platformConfig, this.options, this.volume);
+    if (logs) {
+      if (this.log?.verbosity !== 'silent') {
+        // eslint-disable-next-line no-console
+        console.log('\n' + platform);
+      }
+      for (let logObj of logs) {
+        logObj.success.forEach((success) => {
+          // eslint-disable-next-line no-console
+          console.log(success);
+        });
+        logObj.warning.forEach((warning) => {
+          // eslint-disable-next-line no-console
+          console.log(warning);
+        });
+      }
+    }
     await performActions(dictionary, platformConfig, this.options, this.volume);
     // For chaining
     return this;
@@ -511,8 +523,24 @@ export default class StyleDictionary extends Register {
    */
   async cleanPlatform(platform) {
     const { dictionary, platformConfig } = await this.getPlatform(platform);
-    // We clean files first, then actions, ...and then directories?
-    await cleanFiles(platformConfig, this.volume);
+    // collect logs, cleanFiles happens in parallel but we want to log in sequence
+    const logs = await cleanFiles(platformConfig, this.volume);
+    if (logs) {
+      if (this.log?.verbosity !== 'silent') {
+        // eslint-disable-next-line no-console
+        console.log('\n' + platform);
+      }
+      for (let logObj of logs) {
+        for (let success of logObj.success) {
+          // eslint-disable-next-line no-console
+          console.log(success);
+        }
+        for (let warning of logObj.warning) {
+          // eslint-disable-next-line no-console
+          console.log(warning);
+        }
+      }
+    }
     await cleanActions(dictionary, platformConfig, this.options, this.volume);
     await cleanDirs(platformConfig, this.volume);
     // For chaining

diff --git a/lib/buildFile.js b/lib/buildFile.js
@@ -41,6 +41,11 @@ import createFormatArgs from './utils/createFormatArgs.js';
  * @param {Volume} [vol]
  */
 export default async function buildFile(file, platform = {}, dictionary, options, vol = fs) {
+  /** @type {Record<'warning'|'success', string[]>} */
+  const buildLogs = {
+    warning: [],
+    success: [],
+  };
   const { destination } = file || {};
   const filter = /** @type {Matcher|undefined} */ (file.filter);
   let { format } = file || {};
@@ -81,10 +86,9 @@ export default async function buildFile(file, platform = {}, dictionary, options
     if (platform.log?.warnings === 'error') {
       throw new Error(warnNoFile);
     } else if (platform.log?.verbosity !== 'silent' && platform.log?.warnings !== 'disabled') {
-      // eslint-disable-next-line no-console
-      console.log(chalk.rgb(255, 140, 0)(warnNoFile));
+      buildLogs.warning.push(chalk.rgb(255, 140, 0)(warnNoFile));
     }
-    return null;
+    return buildLogs;
   }
 
   /**
@@ -144,8 +148,7 @@ export default async function buildFile(file, platform = {}, dictionary, options
     filteredReferencesCount === 0 &&
     platform.log?.verbosity !== 'silent'
   ) {
-    // eslint-disable-next-line no-console
-    console.log(chalk.bold.green(`✔︎ ${fullDestination}`));
+    buildLogs.success.push(chalk.bold.green(`✔︎ ${fullDestination}`));
   } else {
     const warnHeader = `⚠️ ${fullDestination}`;
     if (tokenNamesCollisionCount > 0) {
@@ -176,8 +179,7 @@ export default async function buildFile(file, platform = {}, dictionary, options
       if (platform?.log?.warnings === 'error') {
         throw new Error(warn);
       } else if (platform.log?.verbosity !== 'silent' && platform.log?.warnings !== 'disabled') {
-        // eslint-disable-next-line no-console
-        console.log(chalk.rgb(255, 140, 0).bold(warn));
+        buildLogs.warning.push(chalk.rgb(255, 140, 0).bold(warn));
       }
     }
 
@@ -202,9 +204,9 @@ export default async function buildFile(file, platform = {}, dictionary, options
       if (platform?.log?.warnings === 'error') {
         throw new Error(warn);
       } else if (platform.log?.verbosity !== 'silent' && platform.log?.warnings !== 'disabled') {
-        // eslint-disable-next-line no-console
-        console.log(chalk.rgb(255, 140, 0).bold(warn));
+        buildLogs.warning.push(chalk.rgb(255, 140, 0).bold(warn));
       }
     }
   }
+  return buildLogs;
 }
diff --git a/lib/cleanFile.js b/lib/cleanFile.js
@@ -29,8 +29,11 @@ import { fs } from 'style-dictionary/fs';
  * @param {Volume} [vol]
  */
 export default async function cleanFile(file, platform = {}, vol = fs) {
-  // eslint-disable-next-line no-console
-  const consoleLog = platform?.log?.verbosity === 'silent' ? () => {} : console.log;
+  /** @type {Record<'warning'|'success', string[]>} */
+  const cleanLogs = {
+    warning: [],
+    success: [],
+  };
   let { destination } = file;
 
   if (typeof destination !== 'string') throw new Error('Please enter a valid destination');
@@ -40,11 +43,14 @@ export default async function cleanFile(file, platform = {}, vol = fs) {
     destination = platform.buildPath + destination;
   }
 
-  if (!vol.existsSync(destination)) {
-    consoleLog(chalk.bold.red('!') + ' ' + destination + ', does not exist');
-    return;
+  if (!vol.existsSync(destination) && platform?.log?.verbosity !== 'silent') {
+    cleanLogs.success.push(chalk.bold.red('!') + ' ' + destination + ', does not exist');
+    return cleanLogs;
   }
 
   vol.unlinkSync(destination);
-  consoleLog(chalk.bold.red('-') + ' ' + destination);
+  if (platform?.log?.verbosity !== 'silent') {
+    cleanLogs.success.push(chalk.bold.red('-') + ' ' + destination);
+  }
+  return cleanLogs;
 }
diff --git a/package-lock.json b/package-lock.json