From cef23cf549dd3f05e70ec9653ea9d361d53bcd36 Mon Sep 17 00:00:00 2001 From: Simo Kinnunen Date: Fri, 29 Nov 2024 21:12:32 +0900 Subject: [PATCH] feat: optionally bypass runtime dependency checks [sc-22475] (#986) * feat: optionally bypass runtime dependency checks Introduces a new option called `--[no-]verify-runtime-dependencies` for the test and deploy commands, which is true by default and matches current behavior. Should the user decide that they know the available dependencies better than we do, they can run the commands with the `--no-verify-runtime-dependencies` flag set, or they can use the equivalent `CHECKLY_VERIFY_RUNTIME_DEPENDENCIES=0` environment variable. Specifically, this feature makes it possible to use custom dependencies that have been added to a customized private location runtime (contact your Account Executive to learn how). * feat: update tests * fix: enable verifyRuntimeDependencies if not specified in parser options Makes it easier to use parseProject in tests. --- packages/cli/src/commands/deploy.ts | 8 +++ packages/cli/src/commands/test.ts | 8 +++ packages/cli/src/constructs/api-check.ts | 5 +- packages/cli/src/constructs/browser-check.ts | 5 +- .../cli/src/constructs/multi-step-check.ts | 5 +- packages/cli/src/constructs/project.ts | 1 + .../__tests__/check-parser.spec.ts | 61 +++++++++++++++---- .../cli/src/services/check-parser/parser.ts | 19 ++++-- packages/cli/src/services/project-parser.ts | 3 + 9 files changed, 94 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/commands/deploy.ts b/packages/cli/src/commands/deploy.ts index da157a73..07992498 100644 --- a/packages/cli/src/commands/deploy.ts +++ b/packages/cli/src/commands/deploy.ts @@ -55,6 +55,12 @@ export default class Deploy extends AuthCommand { char: 'c', description: commonMessages.configFile, }), + 'verify-runtime-dependencies': Flags.boolean({ + description: '[default: true] Return an error if checks import dependencies that are not supported by the selected runtime.', + default: true, + allowNo: true, + env: 'CHECKLY_VERIFY_RUNTIME_DEPENDENCIES', + }), } async run (): Promise { @@ -66,6 +72,7 @@ export default class Deploy extends AuthCommand { 'schedule-on-deploy': scheduleOnDeploy, output, config: configFilename, + 'verify-runtime-dependencies': verifyRuntimeDependencies, } = flags const { configDirectory, configFilenames } = splitConfigFilePath(configFilename) const { @@ -88,6 +95,7 @@ export default class Deploy extends AuthCommand { acc[runtime.name] = runtime return acc }, > {}), + verifyRuntimeDependencies, checklyConfigConstructs, }) const repoInfo = getGitInformation(project.repoUrl) diff --git a/packages/cli/src/commands/test.ts b/packages/cli/src/commands/test.ts index 280283e2..f6405373 100644 --- a/packages/cli/src/commands/test.ts +++ b/packages/cli/src/commands/test.ts @@ -104,6 +104,12 @@ export default class Test extends AuthCommand { retries: Flags.integer({ description: `[default: 0, max: ${MAX_RETRIES}] How many times to retry a failing test run.`, }), + 'verify-runtime-dependencies': Flags.boolean({ + description: '[default: true] Return an error if checks import dependencies that are not supported by the selected runtime.', + default: true, + allowNo: true, + env: 'CHECKLY_VERIFY_RUNTIME_DEPENDENCIES', + }), } static args = { @@ -137,6 +143,7 @@ export default class Test extends AuthCommand { 'test-session-name': testSessionName, 'update-snapshots': updateSnapshots, retries, + 'verify-runtime-dependencies': verifyRuntimeDependencies, } = flags const filePatterns = argv as string[] @@ -169,6 +176,7 @@ export default class Test extends AuthCommand { acc[runtime.name] = runtime return acc }, > {}), + verifyRuntimeDependencies, checklyConfigConstructs, }) const checks = Object.entries(project.data.check) diff --git a/packages/cli/src/constructs/api-check.ts b/packages/cli/src/constructs/api-check.ts index d293489c..1dd26772 100644 --- a/packages/cli/src/constructs/api-check.ts +++ b/packages/cli/src/constructs/api-check.ts @@ -352,7 +352,10 @@ export class ApiCheck extends Check { if (!runtime) { throw new Error(`${runtimeId} is not supported`) } - const parser = new Parser(Object.keys(runtime.dependencies)) + const parser = new Parser({ + supportedNpmModules: Object.keys(runtime.dependencies), + checkUnsupportedModules: Session.verifyRuntimeDependencies, + }) const parsed = parser.parse(absoluteEntrypoint) // Maybe we can get the parsed deps with the content immediately diff --git a/packages/cli/src/constructs/browser-check.ts b/packages/cli/src/constructs/browser-check.ts index 4c976795..de3ba789 100644 --- a/packages/cli/src/constructs/browser-check.ts +++ b/packages/cli/src/constructs/browser-check.ts @@ -120,7 +120,10 @@ export class BrowserCheck extends Check { if (!runtime) { throw new Error(`${runtimeId} is not supported`) } - const parser = new Parser(Object.keys(runtime.dependencies)) + const parser = new Parser({ + supportedNpmModules: Object.keys(runtime.dependencies), + checkUnsupportedModules: Session.verifyRuntimeDependencies, + }) const parsed = parser.parse(entry) // Maybe we can get the parsed deps with the content immediately diff --git a/packages/cli/src/constructs/multi-step-check.ts b/packages/cli/src/constructs/multi-step-check.ts index a457bc61..41652ca4 100644 --- a/packages/cli/src/constructs/multi-step-check.ts +++ b/packages/cli/src/constructs/multi-step-check.ts @@ -104,7 +104,10 @@ export class MultiStepCheck extends Check { if (!runtime) { throw new Error(`${runtimeId} is not supported`) } - const parser = new Parser(Object.keys(runtime.dependencies)) + const parser = new Parser({ + supportedNpmModules: Object.keys(runtime.dependencies), + checkUnsupportedModules: Session.verifyRuntimeDependencies, + }) const parsed = parser.parse(entry) // Maybe we can get the parsed deps with the content immediately diff --git a/packages/cli/src/constructs/project.ts b/packages/cli/src/constructs/project.ts index 1d472771..8d9d798a 100644 --- a/packages/cli/src/constructs/project.ts +++ b/packages/cli/src/constructs/project.ts @@ -142,6 +142,7 @@ export class Session { static checkFilePath?: string static checkFileAbsolutePath?: string static availableRuntimes: Record + static verifyRuntimeDependencies = true static loadingChecklyConfigFile: boolean static checklyConfigFileConstructs?: Construct[] static privateLocations: PrivateLocationApi[] diff --git a/packages/cli/src/services/check-parser/__tests__/check-parser.spec.ts b/packages/cli/src/services/check-parser/__tests__/check-parser.spec.ts index 83b09428..decf5d73 100644 --- a/packages/cli/src/services/check-parser/__tests__/check-parser.spec.ts +++ b/packages/cli/src/services/check-parser/__tests__/check-parser.spec.ts @@ -9,14 +9,18 @@ const defaultNpmModules = [ describe('dependency-parser - parser()', () => { it('should handle JS file with no dependencies', () => { - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) const { dependencies } = parser.parse(path.join(__dirname, 'check-parser-fixtures', 'no-dependencies.js')) expect(dependencies.map(d => d.filePath)).toHaveLength(0) }) it('should handle JS file with dependencies', () => { const toAbsolutePath = (...filepath: string[]) => path.join(__dirname, 'check-parser-fixtures', 'simple-example', ...filepath) - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) const { dependencies } = parser.parse(toAbsolutePath('entrypoint.js')) expect(dependencies.map(d => d.filePath).sort()).toEqual([ toAbsolutePath('dep1.js'), @@ -31,7 +35,9 @@ describe('dependency-parser - parser()', () => { it('should report a missing entrypoint file', () => { const missingEntrypoint = path.join(__dirname, 'check-parser-fixtures', 'does-not-exist.js') try { - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(missingEntrypoint) } catch (err) { expect(err).toMatchObject({ missingFiles: [missingEntrypoint] }) @@ -41,7 +47,9 @@ describe('dependency-parser - parser()', () => { it('should report missing check dependencies', () => { const toAbsolutePath = (...filepath: string[]) => path.join(__dirname, 'check-parser-fixtures', ...filepath) try { - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(toAbsolutePath('missing-dependencies.js')) } catch (err) { expect(err).toMatchObject({ missingFiles: [toAbsolutePath('does-not-exist.js'), toAbsolutePath('does-not-exist2.js')] }) @@ -51,7 +59,9 @@ describe('dependency-parser - parser()', () => { it('should report syntax errors', () => { const entrypoint = path.join(__dirname, 'check-parser-fixtures', 'syntax-error.js') try { - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(entrypoint) } catch (err) { expect(err).toMatchObject({ parseErrors: [{ file: entrypoint, error: 'Unexpected token (4:70)' }] }) @@ -61,16 +71,29 @@ describe('dependency-parser - parser()', () => { it('should report unsupported dependencies', () => { const entrypoint = path.join(__dirname, 'check-parser-fixtures', 'unsupported-dependencies.js') try { - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(entrypoint) } catch (err) { expect(err).toMatchObject({ unsupportedNpmDependencies: [{ file: entrypoint, unsupportedDependencies: ['left-pad', 'right-pad'] }] }) } }) + it('should allow unsupported dependencies if configured to do so', () => { + const entrypoint = path.join(__dirname, 'check-parser-fixtures', 'unsupported-dependencies.js') + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + checkUnsupportedModules: false, + }) + parser.parse(entrypoint) + }) + it('should handle circular dependencies', () => { const toAbsolutePath = (...filepath: string[]) => path.join(__dirname, 'check-parser-fixtures', 'circular-dependencies', ...filepath) - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) const { dependencies } = parser.parse(toAbsolutePath('entrypoint.js')) // Circular dependencies are allowed in Node.js @@ -84,7 +107,9 @@ describe('dependency-parser - parser()', () => { it('should parse typescript dependencies', () => { const toAbsolutePath = (...filepath: string[]) => path.join(__dirname, 'check-parser-fixtures', 'typescript-example', ...filepath) - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) const { dependencies } = parser.parse(toAbsolutePath('entrypoint.ts')) expect(dependencies.map(d => d.filePath).sort()).toEqual([ toAbsolutePath('dep1.ts'), @@ -102,7 +127,9 @@ describe('dependency-parser - parser()', () => { it('should handle ES Modules', () => { const toAbsolutePath = (...filepath: string[]) => path.join(__dirname, 'check-parser-fixtures', 'esmodules-example', ...filepath) - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) const { dependencies } = parser.parse(toAbsolutePath('entrypoint.js')) expect(dependencies.map(d => d.filePath).sort()).toEqual([ toAbsolutePath('dep1.js'), @@ -113,7 +140,9 @@ describe('dependency-parser - parser()', () => { it('should handle Common JS and ES Modules', () => { const toAbsolutePath = (...filepath: string[]) => path.join(__dirname, 'check-parser-fixtures', 'common-esm-example', ...filepath) - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) const { dependencies } = parser.parse(toAbsolutePath('entrypoint.mjs')) expect(dependencies.map(d => d.filePath).sort()).toEqual([ toAbsolutePath('dep1.js'), @@ -130,7 +159,9 @@ describe('dependency-parser - parser()', () => { */ it.skip('should ignore cases where require is reassigned', () => { const entrypoint = path.join(__dirname, 'check-parser-fixtures', 'reassign-require.js') - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(entrypoint) }) @@ -138,13 +169,17 @@ describe('dependency-parser - parser()', () => { // For consistency with checks created via the UI, the CLI should support this as well. it('should allow top-level await', () => { const entrypoint = path.join(__dirname, 'check-parser-fixtures', 'top-level-await.js') - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(entrypoint) }) it('should allow top-level await in TypeScript', () => { const entrypoint = path.join(__dirname, 'check-parser-fixtures', 'top-level-await.ts') - const parser = new Parser(defaultNpmModules) + const parser = new Parser({ + supportedNpmModules: defaultNpmModules, + }) parser.parse(entrypoint) }) }) diff --git a/packages/cli/src/services/check-parser/parser.ts b/packages/cli/src/services/check-parser/parser.ts index 7625775f..1bb60bda 100644 --- a/packages/cli/src/services/check-parser/parser.ts +++ b/packages/cli/src/services/check-parser/parser.ts @@ -85,13 +85,20 @@ function getTsParser (): any { } } +type ParserOptions = { + supportedNpmModules?: Array + checkUnsupportedModules?: boolean +} + export class Parser { supportedModules: Set + checkUnsupportedModules: boolean // TODO: pass a npm matrix of supported npm modules // Maybe pass a cache so we don't have to fetch files separately all the time - constructor (supportedNpmModules: Array) { - this.supportedModules = new Set([...supportedBuiltinModules, ...supportedNpmModules]) + constructor (options: ParserOptions) { + this.supportedModules = new Set(supportedBuiltinModules.concat(options.supportedNpmModules ?? [])) + this.checkUnsupportedModules = options.checkUnsupportedModules ?? true } parse (entrypoint: string) { @@ -120,9 +127,11 @@ export class Parser { collector.addParsingError(item.filePath, error.message) continue } - const unsupportedDependencies = module.npmDependencies.filter((dep) => !this.supportedModules.has(dep)) - if (unsupportedDependencies.length) { - collector.addUnsupportedNpmDependencies(item.filePath, unsupportedDependencies) + if (this.checkUnsupportedModules) { + const unsupportedDependencies = module.npmDependencies.filter((dep) => !this.supportedModules.has(dep)) + if (unsupportedDependencies.length) { + collector.addUnsupportedNpmDependencies(item.filePath, unsupportedDependencies) + } } const localDependenciesResolvedPaths: Array<{filePath: string, content: string}> = [] module.localDependencies.forEach((localDependency: string) => { diff --git a/packages/cli/src/services/project-parser.ts b/packages/cli/src/services/project-parser.ts index d38c2fe0..1be67ee2 100644 --- a/packages/cli/src/services/project-parser.ts +++ b/packages/cli/src/services/project-parser.ts @@ -23,6 +23,7 @@ type ProjectParseOpts = { checkDefaults?: CheckConfigDefaults, browserCheckDefaults?: CheckConfigDefaults, availableRuntimes: Record, + verifyRuntimeDependencies?: boolean, checklyConfigConstructs?: Construct[], } @@ -43,6 +44,7 @@ export async function parseProject (opts: ProjectParseOpts): Promise { checkDefaults = {}, browserCheckDefaults = {}, availableRuntimes, + verifyRuntimeDependencies, checklyConfigConstructs, } = opts const project = new Project(projectLogicalId, { @@ -57,6 +59,7 @@ export async function parseProject (opts: ProjectParseOpts): Promise { Session.checkDefaults = Object.assign({}, BASE_CHECK_DEFAULTS, checkDefaults) Session.browserCheckDefaults = browserCheckDefaults Session.availableRuntimes = availableRuntimes + Session.verifyRuntimeDependencies = verifyRuntimeDependencies ?? true // TODO: Do we really need all of the ** globs, or could we just put node_modules? const ignoreDirectories = ['**/node_modules/**', '**/.git/**', ...ignoreDirectoriesMatch]