Skip to content

Commit

Permalink
[api-extractor] More predictable merging of arrays in configuration (#…
Browse files Browse the repository at this point in the history
…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 <Josmithr@users.noreply.github.com>
  • Loading branch information
Josmithr and Josmithr authored Feb 11, 2025
1 parent ba27837 commit 7b86640
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 2 deletions.
19 changes: 17 additions & 2 deletions apps/api-extractor/src/api/ExtractorConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,17 @@ export class ExtractorConfig {
let currentConfigFilePath: string = path.resolve(jsonFilePath);
let configObject: Partial<IConfigFile> = {};

// 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.
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand Down
27 changes: 27 additions & 0 deletions apps/api-extractor/src/api/test/ExtractorConfig-merge.test.ts
Original file line number Diff line number Diff line change
@@ -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'
}
]);
});
});
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "./api-extractor-base.json",

"apiReport": {
"reportVariants": ["complete"]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// empty file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "override-array-properties",
"version": "1.0.0"
}
Original file line number Diff line number Diff line change
@@ -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"
}

0 comments on commit 7b86640

Please sign in to comment.