From fc806fced8ef2eecdf38df885462b74b3cb79511 Mon Sep 17 00:00:00 2001 From: Wil Wilsman Date: Tue, 20 Jul 2021 12:23:08 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Ensure=20core=20schemas=20are=20?= =?UTF-8?q?loaded=20before=20dependent=20snapshot=20schemas=20(#431)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✨ Allow replacing existing schemas * ♻ Update schema registration pattern * 🐛 Ensure core schemas are loaded before dependent snapshot schemas --- packages/cli-command/test/command.test.js | 3 +- packages/cli-exec/src/hooks/init.js | 6 ++-- .../cli-snapshot/src/commands/snapshot.js | 6 ++-- packages/cli-snapshot/src/config.js | 7 +++- packages/cli-snapshot/src/hooks/init.js | 13 +++----- packages/cli-upload/src/hooks/init.js | 6 ++-- packages/config/src/validate.js | 14 +++++--- packages/config/test/index.test.js | 33 +++++++++++++++++++ packages/core/src/config.js | 9 ++++- packages/core/src/index.js | 6 +--- 10 files changed, 74 insertions(+), 29 deletions(-) diff --git a/packages/cli-command/test/command.test.js b/packages/cli-command/test/command.test.js index 32e863735..0ad8efb34 100644 --- 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 index 1eeb429c7..bf78ee29e 100644 --- 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 index 14ef73436..3aa81bf7e 100644 --- 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 index 45b95301a..bda6fbbab 100644 --- 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 index 8f0875425..74524f1ed 100644 --- 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 index f04628355..5673d87c8 100644 --- 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 index ac3a603e8..e377d65db 100644 --- 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 index a0d5b3a8c..5003e8825 100644 --- 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 index 029e31489..d3fde653d 100644 --- 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 index ae73abb6e..5d06f7a4e 100644 --- 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;