[api-extractor] Fix a bug where an item is exported from both the entry point and an exported AstNamespaceImport.#3619
Merged
octogonz merged 5 commits intomicrosoft:mainfrom Sep 12, 2022
Conversation
zelliott
commented
Sep 8, 2022
| } | ||
| ] | ||
| }, | ||
| { |
Contributor
Author
There was a problem hiding this comment.
This addition is expected. ForgottenExport6 is exported from an un-exported AstNamespaceImport. It will appear twice in the doc model:
- Once under the un-exported namespace
internal2, with the referenceapi-extractor-scenarios!~internal2.ForgottenExport6:class. - Once under the entry point itself, with the reference
api-extractor-scenarios!~ForgottenExport6:class.
It appears twice because today for largely the same reason items exported from both the entry point and an exported AstNamespaceImport appear twice today. We can improve this behavior in a follow-up PR.
zelliott
commented
Sep 8, 2022
build-tests/api-extractor-scenarios/etc/exportImportStarAs2/api-extractor-scenarios.api.json
Show resolved
Hide resolved
octogonz
approved these changes
Sep 12, 2022
octogonz
reviewed
Sep 12, 2022
| "changes": [ | ||
| { | ||
| "packageName": "@microsoft/api-extractor", | ||
| "comment": "Fix a recent regression where items exported from both the entry point and from an exported namespace appeared only once in the API doc model (GitHub #3619)", |
Collaborator
There was a problem hiding this comment.
@zelliott I updated this change message. Our change log guidelines recommend to use the word "issue" or "regression" instead of "bug".
Collaborator
|
🚀 Released with Thanks for your patience! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a minor bug introduced in #3552.
Details
Suppose you have the following setup:
In the code above,
Foois consumable both from the entry point and from a consumableAstNamespaceImport.Before #3552
Foois written twice to the doc model: once under theApiEntryPoint, once under theApiNamespaceinternal. This is probably not the ideal behavior, but it's what users before #3552 expect.After #3552
When
docModel.includeForgottenExportsis disabled,Foois now only written once to the doc model under theApiNamespace. This is due to some incorrect code withinApiModelGenerator.tsthat does not process an entry point entity if the entity has any parent entities. This code is incorrect because I didn't consider entities that are exported both from the entry point and from anAstNamespaceImport. Instead, this logic should only process entities that are exported from the entry point. This PR implements that fix.Additionally, I fixed a bug that occurs in this scenario when
docModel.includeForgottenExportsis enabled.Previously,
ApiModelGenerator.tsdetermined whether an item is exported based upon whether it was exported from any parent module (viaCollectorEntity.exported). However, this doesn't work if an item is written multiple times to the doc model. This doesn't work because the item's "exported-ness" needs to be evaluated with respect to its respectiveApiItemContainerMixin.Thus. I updated
ApiModelGenerator.tsto passisExportedthrough the process functions. In doing this, I modified the method signatures of the process functions to be more readable by introducing acontextparameter and interface. (because we were passing four arguments into each method).How it was tested
I added two api-extractor-scenario tests:
namespaceImportsandnamespaceImports2:namespaceImports: A scenario where a class is exported from twoAstNamespaceImportnamespaces, and both namespaces are exported from the entry point.namespaceImports2: A scenario where a class is exported from both anAstNamespaceImportand the entry point.These test scenarios are also added in #3602 to validate some declaration reference construction logic. This will make rebasing/coordinating between the two PRs easier.