rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[api-extractor] lodash.merge() counterintuitively applies `$extends` for `reportVariants` setting

Open jason-ha opened this issue 1 year ago • 1 comments

Summary

Using a base configuration with long reportVariants such as

	"apiReport": {
		"reportVariants": ["public", "beta"]
	},

cannot be shortened in a derived config.

If the derived specifies:

	"apiReport": {
		"reportVariants": ["alpha"]
	},

The result using loadash.merge is

	"apiReport": {
		"reportVariants": ["alpha", "beta"]
	},

Workaround

Use array with duplicate entries to mask base config. "reportVariants": ["alpha", "alpha"]

will produce internal "reportConfigs":

  "reportConfigs": [
    {
      "fileName": "fluid-framework.alpha.api.md",
      "variant": "alpha"
    },
    {
      "fileName": "fluid-framework.alpha.api.md",
      "variant": "alpha"
    }
  ],

but won't fail.

Details

Problem occurs in ExtractorConfig.ts line 614:

        // Merge extractorConfig into baseConfig, mutating baseConfig
        lodash.merge(baseConfig, configObject);
        configObject = baseConfig;

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/api-extractor version? 7.45.1
Operating system? Windows
API Extractor scenario? reporting (.api.md)
Would you consider contributing a PR? No
TypeScript compiler version? 5.4.2
Node.js version (node -v)? 18.20.2

jason-ha avatar Jun 15 '24 03:06 jason-ha

Discussed on 9/5/2024: My thinking is that we can safely remove lodash entirely from API Extractor, and replace it with heft-config-file. There might be some minor breaking changes for people using $extends in advanced ways, but I feel this is probably low risk enough that it could even be a minor version bump for API Extractor.

The work item should be relatively easy, since there are only two lodash.merge() calls. Mainly you just have to figure out how to use heft-config-file API; @iclanton is the expert.

octogonz avatar Sep 05 '24 21:09 octogonz

@octogonz PR with a fix is here: https://github.com/microsoft/rushstack/pull/5071

I gave heft-config-file a look, but it seemed fairly involved and I didn't find much in the way of documentation. It would still probably be worth ditching lodash and making merge behaviors more customizable, but the above PR makes a pretty trivial change that should be a significant improvement.

Josmithr avatar Jan 09 '25 00:01 Josmithr