🐛 Ensure core schemas are loaded before dependent snapshot schemas (#431

) * ✨ Allow replacing existing schemas * ♻ Update schema registration pattern * 🐛 Ensure core schemas are loaded before dependent snapshot schemas
percy · Jul 20, 2021 · fc806fc · fc806fc
1 parent d62f5eb
commit fc806fc
Show file tree

Hide file tree

Showing 10 changed files with 74 additions and 29 deletions.
diff --git a/packages/cli-command/test/command.test.js b/packages/cli-command/test/command.test.js
@@ -1,9 +1,10 @@
 import logger from '@percy/logger/test/helpers';
 import PercyConfig from '@percy/config';
+import { configSchema } from '@percy/core/dist/config';
 import PercyCommand, { flags } from '../src';
 
 // add config schema to test discovery flags
-PercyConfig.addSchema(require('@percy/core/dist/config').schema);
+PercyConfig.addSchema(configSchema);
 
 describe('PercyCommand', () => {
   let results;

diff --git a/packages/cli-exec/src/hooks/init.js b/packages/cli-exec/src/hooks/init.js
@@ -1,8 +1,8 @@
 import PercyConfig from '@percy/config';
-import { schema, migration } from '@percy/core/dist/config';
+import * as CoreConfig from '@percy/core/dist/config';
 
 // ensures the core schema and migration is loaded
 export default function() {
-  PercyConfig.addSchema(schema);
-  PercyConfig.addMigration(migration);
+  PercyConfig.addSchema(CoreConfig.schemas);
+  PercyConfig.addMigration(CoreConfig.migration);
 }
diff --git a/packages/cli-snapshot/src/commands/snapshot.js b/packages/cli-snapshot/src/commands/snapshot.js
@@ -7,7 +7,7 @@ import logger from '@percy/logger';
 import globby from 'globby';
 import picomatch from 'picomatch';
 import YAML from 'yaml';
-import { schema } from '../config';
+import { configSchema } from '../config';
 import pkg from '../../package.json';
 
 // Throw a better error message for invalid urls
@@ -43,13 +43,13 @@ export class Snapshot extends Command {
     // static only flags
     files: flags.string({
       description: 'one or more globs matching static file paths to snapshot',
-      default: schema.static.properties.files.default,
+      default: configSchema.static.properties.files.default,
       percyrc: 'static.files',
       multiple: true
     }),
     ignore: flags.string({
       description: 'one or more globs matching static file paths to ignore',
-      default: schema.static.properties.ignore.default,
+      default: configSchema.static.properties.ignore.default,
       percyrc: 'static.ignore',
       multiple: true
     })

diff --git a/packages/cli-snapshot/src/config.js b/packages/cli-snapshot/src/config.js
@@ -1,7 +1,7 @@
 import { snapshotSchema } from '@percy/core/dist/config';
 
 // Config schema for static directories
-export const schema = {
+export const configSchema = {
   static: {
     type: 'object',
     additionalProperties: false,
@@ -67,6 +67,11 @@ export const snapshotListSchema = {
   }]
 };
 
+export const schemas = [
+  configSchema,
+  snapshotListSchema
+];
+
 export function migration(config, { map, del }) {
   /* eslint-disable curly */
   if (config.version < 2) {

diff --git a/packages/cli-snapshot/src/hooks/init.js b/packages/cli-snapshot/src/hooks/init.js
@@ -1,12 +1,9 @@
 import PercyConfig from '@percy/config';
-import {
-  schema,
-  snapshotListSchema,
-  migration
-} from '../config';
+import * as CoreConfig from '@percy/core/dist/config';
+import * as SnapshotConfig from '../config';
 
 export default function() {
-  PercyConfig.addSchema(schema);
-  PercyConfig.addSchema(snapshotListSchema);
-  PercyConfig.addMigration(migration);
+  PercyConfig.addSchema(CoreConfig.schemas);
+  PercyConfig.addSchema(SnapshotConfig.schemas);
+  PercyConfig.addMigration(SnapshotConfig.migration);
 }
diff --git a/packages/cli-upload/src/hooks/init.js b/packages/cli-upload/src/hooks/init.js
@@ -1,7 +1,7 @@
 import PercyConfig from '@percy/config';
-import { schema, migration } from '../config';
+import * as UploadConfig from '../config';
 
 export default function() {
-  PercyConfig.addSchema(schema);
-  PercyConfig.addMigration(migration);
+  PercyConfig.addSchema(UploadConfig.schema);
+  PercyConfig.addMigration(UploadConfig.migration);
 }
diff --git a/packages/config/src/validate.js b/packages/config/src/validate.js
@@ -73,10 +73,16 @@ export function getSchema(name) {
 
 // Adds schemas to the config schema's properties. The config schema is removed, modified, and
 // replaced after the new schemas are added to clear any compiled caches. Existing schemas are
-// removed and replaced as well. If a schema id is provided as the second argument, the schema
-// will be set independently and not added to config schema's properties.
+// removed and replaced as well. If a schema has an existing $id, the schema will not be added
+// as config schema properties.
 export function addSchema(schemas) {
-  if (isArray(schemas) || schemas.$id) {
+  if (isArray(schemas)) {
+    return schemas.map(addSchema);
+  }
+
+  if (schemas.$id) {
+    let { $id } = schemas;
+    if (ajv.getSchema($id)) ajv.removeSchema($id);
     return ajv.addSchema(schemas);
   }
 
@@ -90,7 +96,7 @@ export function addSchema(schemas) {
     ajv.addSchema(schema, $id);
   }
 
-  ajv.addSchema(config, '/config');
+  return ajv.addSchema(config, '/config');
 }
 
 // Resets the schema by removing all schemas and inserting a new default schema.

diff --git a/packages/config/test/index.test.js b/packages/config/test/index.test.js
@@ -118,6 +118,39 @@ describe('PercyConfig', () => {
         message: 'must be a string, received null'
       }]);
     });
+
+    it('can add schemas to replace existing schemas', () => {
+      PercyConfig.addSchema({
+        $id: 'foo',
+        type: 'string'
+      });
+
+      PercyConfig.addSchema({
+        $id: 'foo',
+        type: 'number'
+      });
+
+      expect(PercyConfig.validate('foo', 'foo')).toEqual([{
+        path: '',
+        message: 'must be a number, received a string'
+      }]);
+    });
+
+    it('can add multiple schemas at a time', () => {
+      PercyConfig.addSchema([{
+        foo: { type: 'string' }
+      }, {
+        bar: { $ref: '/config/foo' }
+      }]);
+
+      expect(PercyConfig.validate({
+        foo: 'bar',
+        bar: 100
+      })).toEqual([{
+        path: 'bar',
+        message: 'must be a string, received a number'
+      }]);
+    });
   });
 
   describe('.getDefaults()', () => {

diff --git a/packages/core/src/config.js b/packages/core/src/config.js
@@ -3,7 +3,7 @@ import PercyConfig from '@percy/config';
 import { merge } from '@percy/config/dist/utils';
 
 // Common config options used in Percy commands
-export const schema = {
+export const configSchema = {
   snapshot: {
     type: 'object',
     additionalProperties: false,
@@ -209,6 +209,13 @@ export const snapshotDOMSchema = {
   }
 };
 
+// Convinient reference for schema registration
+export const schemas = [
+  configSchema,
+  snapshotSchema,
+  snapshotDOMSchema
+];
+
 // Migration function
 export function migration(config, { map, del, log }) {
   /* eslint-disable curly */

diff --git a/packages/core/src/index.js b/packages/core/src/index.js
@@ -2,12 +2,8 @@
 const { default: PercyConfig } = require('@percy/config');
 const CoreConfig = require('./config');
 
-PercyConfig.addSchema(CoreConfig.schema);
+PercyConfig.addSchema(CoreConfig.schemas);
 PercyConfig.addMigration(CoreConfig.migration);
 
-// used for per-snapshot validation
-PercyConfig.addSchema(CoreConfig.snapshotSchema);
-PercyConfig.addSchema(CoreConfig.snapshotDOMSchema);
-
 // Export the Percy class with commonjs compatibility
 module.exports = require('./percy').default;