-
Notifications
You must be signed in to change notification settings - Fork 604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[api-documenter] Disambiguate output files with colliding names #1536
Conversation
4d3cf0b
to
69fc925
Compare
69fc925
to
a432cab
Compare
collidingNames?: Set<string> | ||
): void { | ||
if (singletons && collidingNames) { | ||
if (!collidingNames.has(apiItem.displayName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting that we don't have to consider function overloads or nameless items here. I guess it's because DocFX currently doesn't generate pages or TOC entries for them. (The markdown target does, though: example.)
collidingNames.add(apiItem.displayName); | ||
singletons.delete(apiItem.displayName); | ||
// Record the initial entry. | ||
this._collisions.add(collision); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rbuckton Ideally we should implement this in api-extractor-model so other documenters don't have to duplicate this logic. There's already something analogous in ApiItemContainerMixin.findMembersByName().
I won't hold up this PR over it, however, since you said your goal was just to get it working. We'll use #1308 to figure out the longer term solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change to ApiItemContainerMixin.findMembersByName()
turned out to be relatively small, so I went ahead and added it to this PR. Basically I just exposed the lookup as ApiItem.getMergedSiblings()
so we can use it here.
@octogonz Was this PR released as part of 7.4.7 on Sep 25? I'm still having this issue, albeit in my case, something like below and the function docs overriding those of the class: class FBO {}
const fbo = () => new FBO(); I'd have assumed that this is covered by this PR, but maybe that problem is part of #1308? |
@postspectacular this PR introduced disambiguation for the special case of merged declarations. It did not address character casing differences. The overall plan was for the disambiguation to go in two phases: First, we'd apply transformations for special cases, to produce friendlier URLs. But then we'd follow up with a mechanism (e.g. appending numeric suffixes) that would handle problems like character casing, that don't have a friendly solution. Ron was less interested in this part, since he mainly uses the DocFX target, which already has some built-in disambiguation features. I want to get it solved, but I've been spread rather thin lately. BTW if you want to help out, the main blocker is a finalized design. We need a proposal with all the edge cases worked out, like the commented code in #1308 (comment) . Once we agree on a design, actually implementing it should be pretty easy. |
[api-documenter] Disambiguate output files with colliding names
Disambiguates output files for ApiItems for merged declarations. Previously api-documenter would overwrite the same output file when there are different ApiItem entries with different kinds that had the same name. For example:
Previously these would both be output to
c.yml
. Now, when api-documenter detects a collision it appends a suffix to the filename based on the ApiItem'skind
. As a result, the above example would create the filesc-class.yml
andc-interface.yml
.Replaces #1331