From 7b8664099999f29916a3612f10a255fe3afdbdb3 Mon Sep 17 00:00:00 2001 From: Joshua Smithrud <54606601+Josmithr@users.noreply.github.com> Date: Tue, 11 Feb 2025 14:04:50 -0800 Subject: [PATCH] [api-extractor] More predictable merging of arrays in configuration (#5071) * fix(api-extractor): Override lodash's array-merging behavior to make config inheritance intelligible * docs: Add change description * docs: Update comment * test: Better path specification --------- Co-authored-by: Joshua Smithrud --- apps/api-extractor/src/api/ExtractorConfig.ts | 19 +++++++++++-- .../api/test/ExtractorConfig-merge.test.ts | 27 +++++++++++++++++++ .../override-array-properties/README.md | 2 ++ .../api-extractor-base.json | 18 +++++++++++++ .../api-extractor.json | 7 +++++ .../override-array-properties/index.d.ts | 1 + .../override-array-properties/package.json | 4 +++ ...x-config-inheritence_2025-01-09-00-41.json | 10 +++++++ 8 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 apps/api-extractor/src/api/test/ExtractorConfig-merge.test.ts create mode 100644 apps/api-extractor/src/api/test/test-data/override-array-properties/README.md create mode 100644 apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor-base.json create mode 100644 apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor.json create mode 100644 apps/api-extractor/src/api/test/test-data/override-array-properties/index.d.ts create mode 100644 apps/api-extractor/src/api/test/test-data/override-array-properties/package.json create mode 100644 common/changes/@microsoft/api-extractor/josmithr-api-extractor-fix-config-inheritence_2025-01-09-00-41.json diff --git a/apps/api-extractor/src/api/ExtractorConfig.ts b/apps/api-extractor/src/api/ExtractorConfig.ts index 1cdf8c96eb2..986a2f54ff3 100644 --- a/apps/api-extractor/src/api/ExtractorConfig.ts +++ b/apps/api-extractor/src/api/ExtractorConfig.ts @@ -568,6 +568,17 @@ export class ExtractorConfig { let currentConfigFilePath: string = path.resolve(jsonFilePath); let configObject: Partial = {}; + // Lodash merges array values by default, which is unintuitive for config files (and makes it impossible for derived configurations to overwrite arrays). + // For example, given a base config containing an array property with value ["foo", "bar"] and a derived config that specifies ["baz"] for that property, lodash will produce ["baz", "bar"], which is unintuitive. + // This customizer function ensures that arrays are always overwritten. + const mergeCustomizer: lodash.MergeWithCustomizer = (objValue, srcValue) => { + if (Array.isArray(srcValue)) { + return srcValue; + } + // Fall back to default merge behavior. + return undefined; + }; + try { do { // Check if this file was already processed. @@ -612,7 +623,7 @@ export class ExtractorConfig { ExtractorConfig._resolveConfigFileRelativePaths(baseConfig, currentConfigFolderPath); // Merge extractorConfig into baseConfig, mutating baseConfig - lodash.merge(baseConfig, configObject); + lodash.mergeWith(baseConfig, configObject, mergeCustomizer); configObject = baseConfig; currentConfigFilePath = extendsField; @@ -622,7 +633,11 @@ export class ExtractorConfig { } // Lastly, apply the defaults - configObject = lodash.merge(lodash.cloneDeep(ExtractorConfig._defaultConfig), configObject); + configObject = lodash.mergeWith( + lodash.cloneDeep(ExtractorConfig._defaultConfig), + configObject, + mergeCustomizer + ); ExtractorConfig.jsonSchema.validateObject(configObject, jsonFilePath); diff --git a/apps/api-extractor/src/api/test/ExtractorConfig-merge.test.ts b/apps/api-extractor/src/api/test/ExtractorConfig-merge.test.ts new file mode 100644 index 00000000000..ec7d52eed6c --- /dev/null +++ b/apps/api-extractor/src/api/test/ExtractorConfig-merge.test.ts @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as path from 'path'; + +import { ExtractorConfig } from '../ExtractorConfig'; + +const testDataFolder: string = path.join(__dirname, 'test-data'); + +// Tests verifying the merge behavior of ExtractorConfig.loadFile +describe(`${ExtractorConfig.name}.${ExtractorConfig.loadFile.name}`, () => { + it('array properties completely override array properties in the base config', () => { + const extractorConfig: ExtractorConfig = ExtractorConfig.loadFileAndPrepare( + path.join(testDataFolder, 'override-array-properties', 'api-extractor.json') + ); + // Base config specifies: ["alpha", "beta", "public"] + // Derived config specifies: ["complete"] + // By default, lodash's merge() function would generate ["complete", "beta", "public"], + // but we instead want the derived config's array property to completely override that of the base. + expect(extractorConfig.reportConfigs).toEqual([ + { + variant: 'complete', + fileName: 'override-array-properties.api.md' + } + ]); + }); +}); diff --git a/apps/api-extractor/src/api/test/test-data/override-array-properties/README.md b/apps/api-extractor/src/api/test/test-data/override-array-properties/README.md new file mode 100644 index 00000000000..22f9b309e36 --- /dev/null +++ b/apps/api-extractor/src/api/test/test-data/override-array-properties/README.md @@ -0,0 +1,2 @@ +Reproduction of #4786. +Merging of configs should *not* merge arrays - the overriding config file's array properties should completely overwrite the base config's array properties. diff --git a/apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor-base.json b/apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor-base.json new file mode 100644 index 00000000000..f41027c72e4 --- /dev/null +++ b/apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor-base.json @@ -0,0 +1,18 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json", + + "mainEntryPointFilePath": "index.d.ts", + + "apiReport": { + "enabled": true, + "reportVariants": ["alpha", "beta", "public"] + }, + + "docModel": { + "enabled": true + }, + + "dtsRollup": { + "enabled": true + } +} diff --git a/apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor.json b/apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor.json new file mode 100644 index 00000000000..fd4839e09fd --- /dev/null +++ b/apps/api-extractor/src/api/test/test-data/override-array-properties/api-extractor.json @@ -0,0 +1,7 @@ +{ + "extends": "./api-extractor-base.json", + + "apiReport": { + "reportVariants": ["complete"] + } +} diff --git a/apps/api-extractor/src/api/test/test-data/override-array-properties/index.d.ts b/apps/api-extractor/src/api/test/test-data/override-array-properties/index.d.ts new file mode 100644 index 00000000000..4bd8276f349 --- /dev/null +++ b/apps/api-extractor/src/api/test/test-data/override-array-properties/index.d.ts @@ -0,0 +1 @@ +// empty file diff --git a/apps/api-extractor/src/api/test/test-data/override-array-properties/package.json b/apps/api-extractor/src/api/test/test-data/override-array-properties/package.json new file mode 100644 index 00000000000..29ba54f2ce1 --- /dev/null +++ b/apps/api-extractor/src/api/test/test-data/override-array-properties/package.json @@ -0,0 +1,4 @@ +{ + "name": "override-array-properties", + "version": "1.0.0" +} diff --git a/common/changes/@microsoft/api-extractor/josmithr-api-extractor-fix-config-inheritence_2025-01-09-00-41.json b/common/changes/@microsoft/api-extractor/josmithr-api-extractor-fix-config-inheritence_2025-01-09-00-41.json new file mode 100644 index 00000000000..30af3e6d58f --- /dev/null +++ b/common/changes/@microsoft/api-extractor/josmithr-api-extractor-fix-config-inheritence_2025-01-09-00-41.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/api-extractor", + "comment": "Update merge behavior for derived configurations to allow overriding array properties", + "type": "minor" + } + ], + "packageName": "@microsoft/api-extractor" +} \ No newline at end of file