diff --git a/packages/core/src/helpers/options.test.ts b/packages/core/src/helpers/options.test.ts new file mode 100644 index 000000000..6c91de2bf --- /dev/null +++ b/packages/core/src/helpers/options.test.ts @@ -0,0 +1,110 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import type { Logger } from '@dd/core/types'; + +import { resetEnableWarnings, resolveEnable } from './options'; + +const mockLogger: Logger = { + getLogger: jest.fn(), + time: jest.fn() as unknown as Logger['time'], + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +}; + +beforeEach(() => { + jest.clearAllMocks(); + resetEnableWarnings(); +}); + +describe('resolveEnable', () => { + describe('standard boolean / omitted values', () => { + const cases = [ + { + description: 'return false when the config key is undefined', + options: {}, + expected: false, + }, + { + description: 'return false when the config key is null', + options: { myPlugin: null }, + expected: false, + }, + { + description: 'return true when the config key is a truthy object without enable', + options: { myPlugin: { someOther: 'val' } }, + expected: true, + }, + { + description: 'return true when enable is true', + options: { myPlugin: { enable: true } }, + expected: true, + }, + { + description: 'return false when enable is false', + options: { myPlugin: { enable: false } }, + expected: false, + }, + { + description: 'return true when enable is undefined (object present)', + options: { myPlugin: { enable: undefined } }, + expected: true, + }, + ]; + + test.each(cases)('should $description', ({ options, expected }) => { + expect(resolveEnable(options, 'myPlugin', mockLogger)).toBe(expected); + expect(mockLogger.warn).not.toHaveBeenCalled(); + }); + }); + + describe('non-boolean coercion with deprecation warning', () => { + const cases = [ + { + description: 'coerce enable: 1 to true and warn', + options: { myPlugin: { enable: 1 } }, + expected: true, + }, + { + description: 'coerce enable: 0 to false and warn', + options: { myPlugin: { enable: 0 } }, + expected: false, + }, + { + description: 'coerce enable: "true" to true and warn', + options: { myPlugin: { enable: 'true' } }, + expected: true, + }, + { + description: 'coerce enable: "" to false and warn', + options: { myPlugin: { enable: '' } }, + expected: false, + }, + ]; + + test.each(cases)('should $description', ({ options, expected }) => { + expect(resolveEnable(options, 'myPlugin', mockLogger)).toBe(expected); + expect(mockLogger.warn).toHaveBeenCalledTimes(1); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('myPlugin.enable'), + ); + }); + }); + + describe('warn-once behavior', () => { + test('should only warn once per config key across multiple calls', () => { + resolveEnable({ myPlugin: { enable: 1 } }, 'myPlugin', mockLogger); + resolveEnable({ myPlugin: { enable: 'yes' } }, 'myPlugin', mockLogger); + expect(mockLogger.warn).toHaveBeenCalledTimes(1); + }); + + test('should warn separately for different config keys', () => { + resolveEnable({ pluginA: { enable: 1 } }, 'pluginA', mockLogger); + resolveEnable({ pluginB: { enable: 1 } }, 'pluginB', mockLogger); + expect(mockLogger.warn).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/packages/core/src/helpers/options.ts b/packages/core/src/helpers/options.ts new file mode 100644 index 000000000..3a0d35768 --- /dev/null +++ b/packages/core/src/helpers/options.ts @@ -0,0 +1,52 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import type { Logger } from '@dd/core/types'; + +const warnedKeys = new Set(); + +/** + * Resolve the `enable` value for a plugin config key, emitting a deprecation + * warning when the caller passes a non-boolean truthy/falsy value. + * + * Semantics: + * - Config key absent / undefined / falsy → false (plugin disabled). + * - Config key is a truthy object without an `enable` property → true. + * - Config key is a truthy object with `enable` set → coerce to boolean, + * warning once per key if it isn't already a boolean. + */ +export const resolveEnable = ( + options: T, + configKey: C, + log: Logger, +): boolean => { + const pluginConfig = options[configKey]; + + if (pluginConfig && typeof pluginConfig === 'object' && 'enable' in pluginConfig) { + const value = (pluginConfig as Record).enable; + + if (typeof value !== 'boolean' && value !== undefined) { + if (!warnedKeys.has(configKey)) { + warnedKeys.add(configKey); + log.warn( + `\`${configKey}.enable\` should be a boolean, got ${typeof value}. ` + + `Non-boolean values are coerced today but will be rejected in the next major.`, + ); + } + } + + if (value !== undefined) { + // TODO(next major): drop this coercion and reject non-boolean `enable` + // outright. The warning above gives callers one major to migrate. + return !!value; + } + } + + return !!pluginConfig; +}; + +/** @internal Exposed only for tests to reset the warn-once set between cases. */ +export const resetEnableWarnings = (): void => { + warnedKeys.clear(); +}; diff --git a/packages/factory/src/index.test.ts b/packages/factory/src/index.test.ts index 2ca19cce1..f7055f2b5 100644 --- a/packages/factory/src/index.test.ts +++ b/packages/factory/src/index.test.ts @@ -2,6 +2,17 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import type { PluginOptions, Options } from '@dd/core/types'; + +const invokeFactory = async (opts: Options): Promise => { + const { buildPluginFactory } = await import('@dd/factory'); + const factory = buildPluginFactory({ bundler: {}, version: '1.0.0' }); + return factory.raw(opts, { framework: 'esbuild' }) as PluginOptions[]; +}; + +const hasPlugin = (plugins: PluginOptions[], name: string) => + plugins.some((plugin) => plugin.name.includes(name)); + describe('Factory', () => { test('Should not throw with no options', async () => { const { buildPluginFactory } = await import('@dd/factory'); @@ -12,4 +23,47 @@ describe('Factory', () => { factory.vite(); }).not.toThrow(); }); + + describe('enable gating for user-facing plugins', () => { + // The factory is the single source of truth for `.enable`. + // Each user-facing plugin is skipped when its config key is absent or + // explicitly disabled, and included when the config key is present. + + test('Should skip a plugin when its config key is absent', async () => { + const plugins = await invokeFactory({ logLevel: 'none' }); + expect(hasPlugin(plugins, 'output')).toBe(false); + expect(hasPlugin(plugins, 'metrics')).toBe(false); + expect(hasPlugin(plugins, 'rum')).toBe(false); + }); + + test('Should include a plugin when its config key is present', async () => { + const plugins = await invokeFactory({ logLevel: 'none', output: {} }); + expect(hasPlugin(plugins, 'output')).toBe(true); + }); + + test('Should skip a plugin when enable: false', async () => { + const plugins = await invokeFactory({ + logLevel: 'none', + output: { enable: false }, + }); + expect(hasPlugin(plugins, 'output')).toBe(false); + }); + + test('Should include a plugin when enable: true', async () => { + const plugins = await invokeFactory({ + logLevel: 'none', + output: { enable: true }, + }); + expect(hasPlugin(plugins, 'output')).toBe(true); + }); + + test('Should coerce a non-boolean enable value and still include the plugin', async () => { + const plugins = await invokeFactory({ + logLevel: 'none', + // @ts-expect-error - intentional non-boolean to exercise coercion + output: { enable: 1 }, + }); + expect(hasPlugin(plugins, 'output')).toBe(true); + }); + }); }); diff --git a/packages/factory/src/index.ts b/packages/factory/src/index.ts index a1a866a2e..cd3ba8c2b 100644 --- a/packages/factory/src/index.ts +++ b/packages/factory/src/index.ts @@ -34,6 +34,7 @@ import { getContext } from './helpers/context'; import { wrapGetPlugins } from './helpers/wrapPlugins'; import { ALL_ENVS, HOST_NAME } from '@dd/core/constants'; import { notifyOnEnvOverrides } from '@dd/core/helpers/env'; +import { resolveEnable } from '@dd/core/helpers/options'; // #imports-injection-marker import * as apps from '@dd/apps-plugin'; import * as errorTracking from '@dd/error-tracking-plugin'; @@ -160,17 +161,27 @@ export const buildPluginFactory = ({ pluginsToAdd.push(['custom', options.customPlugins]); } - // Add the customer facing plugins. - pluginsToAdd.push( + // Customer-facing plugins are gated by their `.enable` flag. + // Resolving here lets every plugin share the same semantics: + // - config key absent → disabled + // - config key present without `enable` → enabled + // - non-boolean `enable` → coerced, with a single deprecation warning + const userFacingPlugins: [name: string, configKey: string, GetPlugins][] = [ // #configs-injection-marker - ['apps', apps.getPlugins], - ['error-tracking', errorTracking.getPlugins], - ['live-debugger', liveDebugger.getPlugins], - ['metrics', metrics.getPlugins], - ['output', output.getPlugins], - ['rum', rum.getPlugins], + ['apps', apps.CONFIG_KEY, apps.getPlugins], + ['error-tracking', errorTracking.CONFIG_KEY, errorTracking.getPlugins], + ['live-debugger', liveDebugger.CONFIG_KEY, liveDebugger.getPlugins], + ['metrics', metrics.CONFIG_KEY, metrics.getPlugins], + ['output', output.CONFIG_KEY, output.getPlugins], + ['rum', rum.CONFIG_KEY, rum.getPlugins], // #configs-injection-marker - ); + ]; + + for (const [name, configKey, getPlugins] of userFacingPlugins) { + if (resolveEnable(options, configKey, log)) { + pluginsToAdd.push([name, getPlugins]); + } + } // Initialize all our plugins. for (const [name, getPlugins] of pluginsToAdd) { diff --git a/packages/plugins/apps/README.md b/packages/plugins/apps/README.md index 03cdb074a..4490ca1ad 100644 --- a/packages/plugins/apps/README.md +++ b/packages/plugins/apps/README.md @@ -54,10 +54,12 @@ Setting the `apps.dryRun` configuration will override any value set in the envir ### apps.enable -> default: `true` when an `apps` config block is present +> default: `true` when an `apps` config block is present, `false` otherwise. Enable or disable the plugin without removing its configuration. +Must be a boolean. Non-boolean values are coerced today but will be rejected in a future major release. + ### apps.include > default: `[]` diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 86bdec673..0144727cf 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -19,9 +19,6 @@ export type types = { export const getPlugins: GetPlugins = ({ options, context, bundler }) => { const log = context.getLogger(PLUGIN_NAME); const validatedOptions = validateOptions(options); - if (!validatedOptions.enable) { - return []; - } if (context.bundler.name !== 'vite') { log.warn(`The apps plugin only supports Vite; skipping under '${context.bundler.name}'.`); diff --git a/packages/plugins/apps/src/types.ts b/packages/plugins/apps/src/types.ts index f53ac1a6a..c640c6e2c 100644 --- a/packages/plugins/apps/src/types.ts +++ b/packages/plugins/apps/src/types.ts @@ -25,4 +25,7 @@ export type AppsManifest = { }; // We don't enforce identifier, as it needs to be dynamically computed if absent. -export type AppsOptionsWithDefaults = WithRequired; +export type AppsOptionsWithDefaults = Omit< + WithRequired, + 'enable' +>; diff --git a/packages/plugins/apps/src/validate.test.ts b/packages/plugins/apps/src/validate.test.ts index f9fb6af69..a660f1662 100644 --- a/packages/plugins/apps/src/validate.test.ts +++ b/packages/plugins/apps/src/validate.test.ts @@ -5,42 +5,11 @@ import { validateOptions } from '@dd/apps-plugin/validate'; describe('Apps Plugin - validateOptions', () => { - describe('enable flag', () => { - const cases = [ - { - description: 'return false when no apps config is provided', - input: {}, - expected: false, - }, - { - description: 'return true when apps config is an empty object', - input: { apps: {} }, - expected: true, - }, - { - description: 'respect explicit enable true', - input: { apps: { enable: true } }, - expected: true, - }, - { - description: 'respect explicit enable false', - input: { apps: { enable: false } }, - expected: false, - }, - ]; - - test.each(cases)('Should $description', ({ input, expected }) => { - const result = validateOptions(input); - expect(result.enable).toBe(expected); - }); - }); - describe('defaults', () => { test('Should set defaults when nothing is provided', () => { const result = validateOptions({}); expect(result).toEqual({ dryRun: true, - enable: false, include: [], identifier: undefined, name: undefined, @@ -91,7 +60,6 @@ describe('Apps Plugin - validateOptions', () => { expect(result).toEqual({ dryRun: true, - enable: true, include: ['public/**/*', 'dist/**/*'], identifier: 'my-app', name: undefined, diff --git a/packages/plugins/apps/src/validate.ts b/packages/plugins/apps/src/validate.ts index bd1509065..38e75a61e 100644 --- a/packages/plugins/apps/src/validate.ts +++ b/packages/plugins/apps/src/validate.ts @@ -10,15 +10,11 @@ import type { AppsOptions, AppsOptionsWithDefaults } from './types'; export const validateOptions = (options: Options): AppsOptionsWithDefaults => { const resolvedOptions = (options[CONFIG_KEY] || {}) as AppsOptions; - const enable = resolvedOptions.enable ?? !!options[CONFIG_KEY]; - const validatedOptions: AppsOptionsWithDefaults = { - enable, + return { include: resolvedOptions.include || [], dryRun: resolvedOptions.dryRun ?? !getDDEnvValue('APPS_UPLOAD_ASSETS'), identifier: resolvedOptions.identifier?.trim(), name: resolvedOptions.name?.trim() || options.metadata?.name?.trim(), }; - - return validatedOptions; }; diff --git a/packages/plugins/error-tracking/README.md b/packages/plugins/error-tracking/README.md index 3b2ef7786..3e32cb7de 100644 --- a/packages/plugins/error-tracking/README.md +++ b/packages/plugins/error-tracking/README.md @@ -10,6 +10,7 @@ Interact with Error Tracking directly from your build system. - [Configuration](#configuration) + - [errorTracking.enable](#errortrackingenable) - [Sourcemaps Upload](#sourcemaps-upload) - [errorTracking.sourcemaps.bailOnError](#errortrackingsourcemapsbailonerror) - [errorTracking.sourcemaps.dryRun](#errortrackingsourcemapsdryrun) @@ -35,6 +36,14 @@ errorTracking?: { } ``` +### errorTracking.enable + +> default: `true` when an `errorTracking` config block is present, `false` otherwise. + +Enable or disable the plugin without removing its configuration. + +Must be a boolean. Non-boolean values are coerced today but will be rejected in a future major release. + ## Sourcemaps Upload Upload JavaScript sourcemaps to Datadog to un-minify your errors. diff --git a/packages/plugins/error-tracking/src/index.test.ts b/packages/plugins/error-tracking/src/index.test.ts index 507ee846f..5573868d4 100644 --- a/packages/plugins/error-tracking/src/index.test.ts +++ b/packages/plugins/error-tracking/src/index.test.ts @@ -17,17 +17,8 @@ const uploadSourcemapsMock = jest.mocked(uploadSourcemaps); describe('Error Tracking Plugin', () => { describe('getPlugins', () => { - test('Should not initialize the plugin if not enabled', async () => { - expect(getPlugins(getGetPluginsArg({ errorTracking: { enable: false } }))).toHaveLength( - 0, - ); - expect(getPlugins(getGetPluginsArg())).toHaveLength(0); - }); - - test('Should initialize the plugin if enabled', async () => { - expect( - getPlugins(getGetPluginsArg({ errorTracking: { enable: true } })).length, - ).toBeGreaterThan(0); + test('Should initialize the plugin', async () => { + expect(getPlugins(getGetPluginsArg({ errorTracking: {} })).length).toBeGreaterThan(0); }); }); diff --git a/packages/plugins/error-tracking/src/index.ts b/packages/plugins/error-tracking/src/index.ts index 2b5b2b4c1..5d8d353b3 100644 --- a/packages/plugins/error-tracking/src/index.ts +++ b/packages/plugins/error-tracking/src/index.ts @@ -19,16 +19,10 @@ export type types = { export const getPlugins: GetPlugins = ({ options, context }) => { const log = context.getLogger(PLUGIN_NAME); - // Verify configuration. const timeOptions = log.time('validate options'); const validatedOptions = validateOptions(options, log); timeOptions.end(); - // If the plugin is not enabled, return an empty array. - if (!validatedOptions.enable) { - return []; - } - let gitInfo: RepositoryData | undefined; let buildReport: BuildReport | undefined; let sourcemapsHandled: boolean = false; diff --git a/packages/plugins/error-tracking/src/types.ts b/packages/plugins/error-tracking/src/types.ts index 018350600..87e8f5e73 100644 --- a/packages/plugins/error-tracking/src/types.ts +++ b/packages/plugins/error-tracking/src/types.ts @@ -23,12 +23,10 @@ export type ErrorTrackingOptions = { }; export type ErrorTrackingOptionsWithDefaults = { - enable?: boolean; sourcemaps?: SourcemapsOptionsWithDefaults; }; export type ErrorTrackingOptionsWithSourcemaps = { - enable?: boolean; sourcemaps: SourcemapsOptionsWithDefaults; }; diff --git a/packages/plugins/error-tracking/src/validate.test.ts b/packages/plugins/error-tracking/src/validate.test.ts index a657672d7..ce8203709 100644 --- a/packages/plugins/error-tracking/src/validate.test.ts +++ b/packages/plugins/error-tracking/src/validate.test.ts @@ -4,9 +4,13 @@ import type { SourcemapsOptions } from '@dd/error-tracking-plugin/types'; import { validateOptions, validateSourcemapsOptions } from '@dd/error-tracking-plugin/validate'; -import { mockLogger, getMinimalSourcemapsConfiguration } from '@dd/tests/_jest/helpers/mocks'; +import { getMinimalSourcemapsConfiguration, mockLogger } from '@dd/tests/_jest/helpers/mocks'; import stripAnsi from 'strip-ansi'; +beforeEach(() => { + jest.clearAllMocks(); +}); + describe('Error Tracking Plugins validate', () => { describe('validateOptions', () => { test('Should return the validated configuration', () => { @@ -15,15 +19,13 @@ describe('Error Tracking Plugins validate', () => { auth: { apiKey: '123', }, - errorTracking: { - enable: true, - }, + errorTracking: {}, }, mockLogger, ); expect(config).toEqual({ - enable: true, + sourcemaps: undefined, }); }); @@ -44,6 +46,7 @@ describe('Error Tracking Plugins validate', () => { }).toThrow(); }); }); + describe('validateSourcemapsOptions', () => { test('Should return errors for each missing required field', () => { const { errors } = validateSourcemapsOptions({ diff --git a/packages/plugins/error-tracking/src/validate.ts b/packages/plugins/error-tracking/src/validate.ts index 8afffe494..f96b9241d 100644 --- a/packages/plugins/error-tracking/src/validate.ts +++ b/packages/plugins/error-tracking/src/validate.ts @@ -26,19 +26,10 @@ export const validateOptions = (config: Options, log: Logger): ErrorTrackingOpti throw new Error(`Invalid configuration for ${PLUGIN_NAME}.`); } - // Build the final configuration. - const toReturn: ErrorTrackingOptionsWithDefaults = { - enable: !!config[CONFIG_KEY], + return { ...config[CONFIG_KEY], - sourcemaps: undefined, + sourcemaps: sourcemapsResults.config, }; - - // Fill in the defaults. - if (sourcemapsResults.config) { - toReturn.sourcemaps = sourcemapsResults.config; - } - - return toReturn; }; type ToReturn = { diff --git a/packages/plugins/live-debugger/README.md b/packages/plugins/live-debugger/README.md index e7a246bd6..90a517333 100644 --- a/packages/plugins/live-debugger/README.md +++ b/packages/plugins/live-debugger/README.md @@ -123,10 +123,12 @@ const double = (x) => { ### liveDebugger.enable -> default: `true` when a `liveDebugger` config block is present +> default: `true` when a `liveDebugger` config block is present, `false` otherwise. Enable or disable the plugin without removing its configuration. +Must be a boolean. Non-boolean values are coerced today but will be rejected in a future major release. + ### metadata.version > default: `undefined` diff --git a/packages/plugins/live-debugger/src/index.test.ts b/packages/plugins/live-debugger/src/index.test.ts index 6c42934db..2bbd7f5fa 100644 --- a/packages/plugins/live-debugger/src/index.test.ts +++ b/packages/plugins/live-debugger/src/index.test.ts @@ -14,7 +14,6 @@ import type { LiveDebuggerOptionsWithDefaults } from './types'; const makeOptions = ( overrides: Partial = {}, ): LiveDebuggerOptionsWithDefaults => ({ - enable: true, version: '1.0.0', include: [/\.[jt]sx?$/], exclude: [/\/node_modules\//], @@ -409,24 +408,6 @@ describe('getLiveDebuggerPlugin', () => { }); describe('getPlugins', () => { - it('should return an empty array when enable is false', () => { - const arg = getGetPluginsArg({ liveDebugger: { enable: false } }); - - const plugins = getPlugins(arg); - - expect(plugins).toEqual([]); - expect(arg.context.inject).not.toHaveBeenCalled(); - }); - - it('should return an empty array when liveDebugger config is omitted', () => { - const arg = getGetPluginsArg(); - - const plugins = getPlugins(arg); - - expect(plugins).toEqual([]); - expect(arg.context.inject).not.toHaveBeenCalled(); - }); - it('should inject runtime stubs and return a plugin when an empty config is provided', () => { const arg = getGetPluginsArg({ liveDebugger: {} }); diff --git a/packages/plugins/live-debugger/src/index.ts b/packages/plugins/live-debugger/src/index.ts index fd93e5f6b..88f90a3e4 100644 --- a/packages/plugins/live-debugger/src/index.ts +++ b/packages/plugins/live-debugger/src/index.ts @@ -152,10 +152,6 @@ export const getPlugins: GetPlugins = ({ options, context }) => { const log = context.getLogger(PLUGIN_NAME); const validatedOptions = validateOptions(options, log); - if (!validatedOptions.enable) { - return []; - } - // Inject no-op stubs for the runtime globals so instrumented code // doesn't crash when the Datadog Browser Debugger SDK is absent. // The SDK's init() overwrites these with the real implementations. diff --git a/packages/plugins/live-debugger/src/types.ts b/packages/plugins/live-debugger/src/types.ts index 98afc0bc9..f6d69074a 100644 --- a/packages/plugins/live-debugger/src/types.ts +++ b/packages/plugins/live-debugger/src/types.ts @@ -23,7 +23,6 @@ export type LiveDebuggerOptions = { }; export type LiveDebuggerOptionsWithDefaults = { - enable: boolean; version: string | undefined; include: (string | RegExp)[]; exclude: (string | RegExp)[]; diff --git a/packages/plugins/live-debugger/src/validate.test.ts b/packages/plugins/live-debugger/src/validate.test.ts index d08219a28..09a282a58 100644 --- a/packages/plugins/live-debugger/src/validate.test.ts +++ b/packages/plugins/live-debugger/src/validate.test.ts @@ -30,10 +30,9 @@ describe('validateOptions', () => { describe('defaults', () => { const cases = [ { - description: 'disable when no options are provided', + description: 'return defaults when no options are provided', input: makeConfig(undefined), expected: { - enable: false, version: undefined, include: [/\.[jt]sx?$/], exclude: expect.arrayContaining([/\/node_modules\//]), @@ -43,20 +42,9 @@ describe('validateOptions', () => { } satisfies LiveDebuggerOptionsWithDefaults, }, { - description: 'honor enable: false even when the config key is present', - input: makeConfig({ enable: false }), - expected: expect.objectContaining({ enable: false, version: undefined }), - }, - { - description: 'honor enable: false even when metadata.version is provided', - input: makeConfig({ enable: false }, { version: '1.0.0' }), - expected: expect.objectContaining({ enable: false, version: '1.0.0' }), - }, - { - description: 'enable and return defaults when an empty object is provided', + description: 'return defaults when an empty object is provided', input: makeConfig({}), expected: { - enable: true, version: undefined, include: [/\.[jt]sx?$/], exclude: expect.arrayContaining([/\/node_modules\//]), @@ -66,15 +54,9 @@ describe('validateOptions', () => { } satisfies LiveDebuggerOptionsWithDefaults, }, { - description: 'honor enable: true and forward metadata.version', - input: makeConfig({ enable: true }, { version: '1.0.0' }), - expected: expect.objectContaining({ enable: true, version: '1.0.0' }), - }, - { - description: 'enable and forward metadata.version when liveDebugger is empty', + description: 'forward metadata.version when liveDebugger is empty', input: makeConfig({}, { version: '1.0.0' }), expected: { - enable: true, version: '1.0.0', include: [/\.[jt]sx?$/], exclude: expect.arrayContaining([/\/node_modules\//]), @@ -86,12 +68,12 @@ describe('validateOptions', () => { { description: 'leave version undefined when metadata is omitted', input: makeConfig({}), - expected: expect.objectContaining({ enable: true, version: undefined }), + expected: expect.objectContaining({ version: undefined }), }, { description: 'leave version undefined when only metadata.name is set', input: makeConfig({}, { name: 'my-build' }), - expected: expect.objectContaining({ enable: true, version: undefined }), + expected: expect.objectContaining({ version: undefined }), }, ]; @@ -258,28 +240,6 @@ describe('validateOptions', () => { }); }); - describe('invalid enable', () => { - const cases = [ - { - description: 'reject enable when a string', - input: makeInvalidConfig({ enable: 'yes' }), - }, - { - description: 'reject enable when a number', - input: makeInvalidConfig({ enable: 1 }), - }, - ]; - - test.each(cases)('should $description', ({ input }) => { - expect(() => validateOptions(input, mockLogger)).toThrow( - `Invalid configuration for ${PLUGIN_NAME}.`, - ); - expect(mockError).toHaveBeenCalledWith( - expect.stringMatching(/enable.*must be a boolean/), - ); - }); - }); - describe('invalid honorSkipComments', () => { const cases = [ { @@ -349,7 +309,6 @@ describe('validateOptions', () => { describe('multiple errors', () => { it('should aggregate all validation errors before throwing', () => { const input = makeInvalidConfig({ - enable: 'yes', include: 'bad', exclude: 'bad', honorSkipComments: 42, @@ -362,7 +321,6 @@ describe('validateOptions', () => { ); const errorMessage = mockError.mock.calls[0][0]; - expect(errorMessage).toMatch(/enable/); expect(errorMessage).toMatch(/include/); expect(errorMessage).toMatch(/exclude/); expect(errorMessage).toMatch(/honorSkipComments/); diff --git a/packages/plugins/live-debugger/src/validate.ts b/packages/plugins/live-debugger/src/validate.ts index ede13beee..eb8020792 100644 --- a/packages/plugins/live-debugger/src/validate.ts +++ b/packages/plugins/live-debugger/src/validate.ts @@ -16,11 +16,6 @@ export const validateOptions = (config: Options, log: Logger): LiveDebuggerOptio const metadataVersion = config.metadata?.version; const errors: string[] = []; - // Validate enable option - if (pluginConfig.enable !== undefined && typeof pluginConfig.enable !== 'boolean') { - errors.push(`${red('enable')} must be a boolean`); - } - // Validate include option if (pluginConfig.include !== undefined) { if (!Array.isArray(pluginConfig.include)) { @@ -86,7 +81,6 @@ export const validateOptions = (config: Options, log: Logger): LiveDebuggerOptio // Build the final configuration with defaults return { - enable: pluginConfig.enable ?? !!config[CONFIG_KEY], version: metadataVersion, include: pluginConfig.include || [/\.[jt]sx?$/], // .js, .jsx, .ts, .tsx exclude: pluginConfig.exclude || [ diff --git a/packages/plugins/metrics/README.md b/packages/plugins/metrics/README.md index cef665120..50781de50 100644 --- a/packages/plugins/metrics/README.md +++ b/packages/plugins/metrics/README.md @@ -47,9 +47,11 @@ metrics?: { ### `enable` -> default: `true` +> default: `true` when a `metrics` config block is present, `false` otherwise. + +Enable or disable the plugin without removing its configuration. -Plugin will be enabled and track metrics when set to `true`. +Must be a boolean. Non-boolean values are coerced today but will be rejected in a future major release. ### `enableDefaultPrefix` diff --git a/packages/plugins/metrics/src/common/helpers.test.ts b/packages/plugins/metrics/src/common/helpers.test.ts index 1ce0e9418..325a35769 100644 --- a/packages/plugins/metrics/src/common/helpers.test.ts +++ b/packages/plugins/metrics/src/common/helpers.test.ts @@ -23,7 +23,6 @@ describe('Metrics Helpers', () => { test('Should return the default options', () => { const options = { ...defaultPluginOptions, [CONFIG_KEY]: {} }; expect(validateOptions(options, 'webpack')).toEqual({ - enable: true, enableDefaultPrefix: true, enableTracing: false, filters: defaultFilters, @@ -38,7 +37,6 @@ describe('Metrics Helpers', () => { const options = { ...defaultPluginOptions, [CONFIG_KEY]: { - enable: false, enableTracing: true, filters: [fakeFilter], prefix: 'prefix', @@ -46,7 +44,6 @@ describe('Metrics Helpers', () => { }, }; expect(validateOptions(options, 'webpack')).toEqual({ - enable: false, enableDefaultPrefix: true, enableTracing: true, filters: [fakeFilter], diff --git a/packages/plugins/metrics/src/common/helpers.ts b/packages/plugins/metrics/src/common/helpers.ts index d7d136250..9df625178 100644 --- a/packages/plugins/metrics/src/common/helpers.ts +++ b/packages/plugins/metrics/src/common/helpers.ts @@ -31,7 +31,6 @@ export const validateOptions = ( } return { - enable: !!opts[CONFIG_KEY], enableDefaultPrefix: true, enableTracing: false, filters: defaultFilters, diff --git a/packages/plugins/metrics/src/index.test.ts b/packages/plugins/metrics/src/index.test.ts index ce32b8619..3859b6f7b 100644 --- a/packages/plugins/metrics/src/index.test.ts +++ b/packages/plugins/metrics/src/index.test.ts @@ -93,15 +93,8 @@ describe('Metrics Universal Plugin', () => { }); describe('getPlugins', () => { - test('Should not initialize the plugin if not enabled', async () => { - expect(getPlugins(getGetPluginsArg({ metrics: { enable: false } }))).toHaveLength(0); - expect(getPlugins(getGetPluginsArg())).toHaveLength(0); - }); - - test('Should initialize the plugin if enabled', async () => { - expect( - getPlugins(getGetPluginsArg({ metrics: { enable: true } })).length, - ).toBeGreaterThan(0); + test('Should initialize the plugin', async () => { + expect(getPlugins(getGetPluginsArg({ metrics: {} })).length).toBeGreaterThan(0); }); }); diff --git a/packages/plugins/metrics/src/index.ts b/packages/plugins/metrics/src/index.ts index 8eeb3bc12..209ae79c5 100644 --- a/packages/plugins/metrics/src/index.ts +++ b/packages/plugins/metrics/src/index.ts @@ -33,11 +33,6 @@ export const getPlugins: GetPlugins = ({ options, context }) => { const validatedOptions = validateOptions(options, context.bundler.name); const plugins: PluginOptions[] = []; - // If the plugin is not enabled, return an empty array. - if (!validatedOptions.enable) { - return plugins; - } - // Webpack and Esbuild specific plugins. // LEGACY const legacyPlugin: PluginOptions = { diff --git a/packages/plugins/metrics/src/types.ts b/packages/plugins/metrics/src/types.ts index 4ef873f06..2fa413e7d 100644 --- a/packages/plugins/metrics/src/types.ts +++ b/packages/plugins/metrics/src/types.ts @@ -17,7 +17,7 @@ export type MetricsOptions = { timestamp?: number; }; -export type MetricsOptionsWithDefaults = Required; +export type MetricsOptionsWithDefaults = Required>; export interface ModuleGraph { getModule(dependency: Dependency): Module; diff --git a/packages/plugins/output/README.md b/packages/plugins/output/README.md index 4efa1f7ba..f8ba2cc34 100644 --- a/packages/plugins/output/README.md +++ b/packages/plugins/output/README.md @@ -37,9 +37,11 @@ output?: { ### `enable` -> default: `true` +> default: `true` when an `output` config block is present, `false` otherwise. -Enable or disable the output plugin. +Enable or disable the plugin without removing its configuration. + +Must be a boolean. Non-boolean values are coerced today but will be rejected in a future major release. ### `path` diff --git a/packages/plugins/output/src/index.test.ts b/packages/plugins/output/src/index.test.ts index de9c6be2d..ce687f861 100644 --- a/packages/plugins/output/src/index.test.ts +++ b/packages/plugins/output/src/index.test.ts @@ -17,13 +17,8 @@ const mockedOutputJson = jest.mocked(outputJson); describe('Output Plugin', () => { describe('getPlugins', () => { - test('Should not initialize the plugin if not enabled', async () => { - expect(getPlugins(getGetPluginsArg({ output: { enable: false } }))).toHaveLength(0); - expect(getPlugins(getGetPluginsArg())).toHaveLength(0); - }); - - test('Should initialize the plugin if enabled', async () => { - expect(getPlugins(getGetPluginsArg({ output: { enable: true } }))).toHaveLength(1); + test('Should initialize the plugin', async () => { + expect(getPlugins(getGetPluginsArg({ output: {} }))).toHaveLength(1); }); }); diff --git a/packages/plugins/output/src/index.ts b/packages/plugins/output/src/index.ts index e366494c4..26343a312 100644 --- a/packages/plugins/output/src/index.ts +++ b/packages/plugins/output/src/index.ts @@ -87,14 +87,8 @@ export const getFilePath = (outDir: string, pathOption: string, filename: string }; export const getPlugins: GetPlugins = ({ options, context }) => { - // Verify configuration. - const validatedOptions = validateOptions(options); const log = context.getLogger(PLUGIN_NAME); - - // If the plugin is not enabled, return an empty array. - if (!validatedOptions.enable) { - return []; - } + const validatedOptions = validateOptions(options); const writeFile = (name: FileKey, data: any) => { const fileValue: FileValue = validatedOptions.files[name]; diff --git a/packages/plugins/output/src/types.ts b/packages/plugins/output/src/types.ts index acdb5b698..4683f53c2 100644 --- a/packages/plugins/output/src/types.ts +++ b/packages/plugins/output/src/types.ts @@ -18,7 +18,7 @@ export type OutputOptions = { }; export type OutputOptionsWithDefaults = Assign< - Required, + Required>, { files: { [K in FileKey]: DefaultFileValue; diff --git a/packages/plugins/output/src/validate.test.ts b/packages/plugins/output/src/validate.test.ts index e1b8710bf..27171230c 100644 --- a/packages/plugins/output/src/validate.test.ts +++ b/packages/plugins/output/src/validate.test.ts @@ -5,36 +5,6 @@ import { validateOptions } from './validate'; describe('validateOptions', () => { - describe('enable', () => { - const cases = [ - { - description: 'return false when no output config provided', - input: {}, - expected: false, - }, - { - description: 'return true when output config is an empty object', - input: { output: {} }, - expected: true, - }, - { - description: 'return true when output config has enable: true', - input: { output: { enable: true } }, - expected: true, - }, - { - description: 'return false when output config has enable: false', - input: { output: { enable: false } }, - expected: false, - }, - ]; - - test.each(cases)('Should $description', ({ input, expected }) => { - const result = validateOptions(input); - expect(result.enable).toBe(expected); - }); - }); - describe('path', () => { const cases = [ { diff --git a/packages/plugins/output/src/validate.ts b/packages/plugins/output/src/validate.ts index c99390989..81bc824e5 100644 --- a/packages/plugins/output/src/validate.ts +++ b/packages/plugins/output/src/validate.ts @@ -41,13 +41,9 @@ const validateFilesOptions = ( // Deal with validation and defaults here. export const validateOptions = (options: Options): OutputOptionsWithDefaults => { - const validatedOptions: OutputOptionsWithDefaults = { - // By using an empty object, we consider the plugin as enabled. - enable: !!options[CONFIG_KEY], + return { path: './', ...options[CONFIG_KEY], files: validateFilesOptions(options[CONFIG_KEY]?.files), }; - - return validatedOptions; }; diff --git a/packages/plugins/rum/src/index.test.ts b/packages/plugins/rum/src/index.test.ts index 8fe18ad6d..35306fab8 100644 --- a/packages/plugins/rum/src/index.test.ts +++ b/packages/plugins/rum/src/index.test.ts @@ -53,28 +53,11 @@ describe('RUM Plugin', () => { ]; describe('getPlugins', () => { const injectMock = jest.fn(); - test('Should not initialize the plugin if disabled', async () => { + test('Should initialize the plugin', async () => { getPlugins( getGetPluginsArg( { rum: { - enable: false, - sdk: { applicationId: 'app-id', clientToken: '123' }, - }, - }, - { inject: injectMock }, - ), - ); - getPlugins(getGetPluginsArg({}, { inject: injectMock })); - expect(injectMock).not.toHaveBeenCalled(); - }); - - test('Should initialize the plugin if enabled', async () => { - getPlugins( - getGetPluginsArg( - { - rum: { - enable: true, sdk: { applicationId: 'app-id', clientToken: '123' }, }, }, diff --git a/packages/plugins/rum/src/index.ts b/packages/plugins/rum/src/index.ts index 0bf029795..fbeafdb2c 100644 --- a/packages/plugins/rum/src/index.ts +++ b/packages/plugins/rum/src/index.ts @@ -28,15 +28,9 @@ export type types = { export const getPlugins: GetPlugins = ({ options, context }) => { const log = context.getLogger(PLUGIN_NAME); - // Verify configuration. const validatedOptions = validateOptions(options, log); const plugins: PluginOptions[] = []; - // If the plugin is not enabled, return an empty array. - if (!validatedOptions.enable) { - return plugins; - } - if (validatedOptions.sourceCodeContext) { context.inject({ type: 'code', diff --git a/packages/plugins/rum/src/types.ts b/packages/plugins/rum/src/types.ts index 1a8e6bfd6..23986ca9c 100644 --- a/packages/plugins/rum/src/types.ts +++ b/packages/plugins/rum/src/types.ts @@ -60,7 +60,6 @@ export type SDKOptionsWithDefaults = Assign< >; export type RumOptionsWithDefaults = { - enable?: boolean; sdk?: SDKOptionsWithDefaults; privacy?: PrivacyOptionsWithDefaults; sourceCodeContext?: SourceCodeContextOptions; diff --git a/packages/plugins/rum/src/validate.ts b/packages/plugins/rum/src/validate.ts index 3fe8a8603..ccaa994d5 100644 --- a/packages/plugins/rum/src/validate.ts +++ b/packages/plugins/rum/src/validate.ts @@ -38,7 +38,6 @@ export const validateOptions = ( // Build the final configuration. const toReturn: RumOptionsWithDefaults = { - enable: !!options[CONFIG_KEY], ...options[CONFIG_KEY], sdk: undefined, privacy: undefined, diff --git a/packages/tools/src/commands/create-plugin/templates.ts b/packages/tools/src/commands/create-plugin/templates.ts index ba14c5621..737dcaa7c 100644 --- a/packages/tools/src/commands/create-plugin/templates.ts +++ b/packages/tools/src/commands/create-plugin/templates.ts @@ -45,23 +45,14 @@ export const getFiles = (context: Context): File[] => { // Deal with validation and defaults here. export const validateOptions = (options: Options): ${pascalCase}OptionsWithDefaults => { - const validatedOptions: ${pascalCase}OptionsWithDefaults = { - // By using an empty object, we consider the plugin as enabled. - enable: !!options[CONFIG_KEY], + return { ...options[CONFIG_KEY] }; - return validatedOptions; }; export const getPlugins: GetPlugins = ({ options, context }) => { - // Verify configuration. const validatedOptions = validateOptions(options); - // If the plugin is not enabled, return an empty array. - if (!validatedOptions.enable) { - return []; - } - const log = context.getLogger(PLUGIN_NAME); return [ @@ -82,7 +73,7 @@ export const getFiles = (context: Context): File[] => { enable?: boolean; }; - export type ${pascalCase}OptionsWithDefaults = Required<${pascalCase}Options>; + export type ${pascalCase}OptionsWithDefaults = Required>; `; }, }, @@ -95,12 +86,7 @@ export const getFiles = (context: Context): File[] => { describe('${title} Plugin', () => { describe('getPlugins', () => { - test('Should not initialize the plugin if not enabled', async () => { - expect(getPlugins(getGetPluginsArg({ ${camelCase}: { enable: false } }))).toHaveLength(0); - expect(getPlugins(getGetPluginsArg())).toHaveLength(0); - }); - - test('Should initialize the plugin if enabled', async () => { + test('Should initialize the plugin', async () => { expect(getPlugins(getGetPluginsArg({ ${camelCase}: { enable: true } }))).toHaveLength(1); }); }); diff --git a/packages/tools/src/commands/integrity/files.ts b/packages/tools/src/commands/integrity/files.ts index a682c1912..3269c5e0c 100644 --- a/packages/tools/src/commands/integrity/files.ts +++ b/packages/tools/src/commands/integrity/files.ts @@ -101,7 +101,7 @@ const updateFactory = async (plugins: Workspace[]) => { import * as ${camelCase} from '${plugin.name}'; `; typesExportContent += `export type { types as ${pascalCase}Types } from '${plugin.name}';`; - configContent += `['${cleanPluginName(plugin.name)}', ${camelCase}.getPlugins],`; + configContent += `['${cleanPluginName(plugin.name)}', ${configKeyVar}, ${camelCase}.getPlugins],`; // Only add helpers if they export them. if (pluginExports.helpers && Object.keys(pluginExports.helpers).length) {