Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor(compiler): Allow extraction for some private symbols, like $… #55340

Closed
wants to merge 1 commit into from
Closed

refactor(compiler): Allow extraction for some private symbols, like $… #55340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/compiler-cli/src/ngtsc/docs/src/extractor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import {extractTypeAlias} from './type_alias_extractor';

type DeclarationWithExportName = readonly[string, ts.Declaration];

const exportedAsGlobals = new Map([
[`ɵ$localize`, '$localize'], // exported as private but available as global
]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to clarify whether we want to hard-code this FW-repo specific logic into ngtsc/docs. My understanding is that the docs extraction can be a useful tool for other pipelines as well, in the future.

A more future-proof and also code-local solution would be adding special tag information to these exports. e.g. @forceApiGeneration

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or idealy, understand why #55345 doesn't work ?

Having forceApiGeneration would still require us to rename the symbol to remove the bared o.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would also work. In either case, not a big fan of a map in ngtsc/docs. The JSDoc could also include the name that it should be renamed to. e.g. @forceApiGeneration $localize

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like we should fix the root cause though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW there are already framework-specific patterns hard-coded into the docs extraction (though not as far as hard-coding specific symbols). If we ever want to open it up to code outside the Angular team, it would be a whole project to deal with this. I do agree, though, that it's better to avoid hard-coding things if we have a reasonable, more general alternative.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even aside from hard-coding specifics (while the current ones, as you said, are not symbol specific, except maybe Angular-ecosystem wide lifecycle hooks), keeping these "special behaviors" local to the exported declarations is cleaner and more obvious.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that if we can get #55345 to work, that would be better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was finally able to get what I wanted in #55345.

/**
* Extracts all information from a source file that may be relevant for generating
* public API documentation.
Expand All @@ -42,15 +45,17 @@ export class DocsExtractor {
const exportedDeclarations = this.getExportedDeclarations(sourceFile);
for (const [exportName, node] of exportedDeclarations) {
// Skip any symbols with an Angular-internal name.
if (isAngularPrivateName(exportName)) {

const allowedPrivate = exportedAsGlobals.get(exportName);
if (isAngularPrivateName(exportName) && !allowedPrivate) {
continue;
}

const entry = this.extractDeclaration(node);
if (entry && !isIgnoredDocEntry(entry)) {
// The exported name of an API may be different from its declaration name, so
// use the declaration name.
entries.push({...entry, name: exportName});
entries.push({...entry, name: allowedPrivate ?? exportName});
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/localize/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ generate_api_docs(
name = "localize_docs",
srcs = [
":files_for_docgen",
"//packages/localize/src/localize:files_for_docgen",
"//packages/localize/src/utils:files_for_docgen",
],
entry_point = ":index.ts",
Expand Down