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.

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

fix(compiler-cli): avoid broken references in .d.ts files due to @internal markers #43527

Closed
wants to merge 3 commits into from

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Sep 21, 2021

The ErrorCode enum in the error_code.ts file is governed by public
api guards but the other top-level exports from that file are exempt
from public api documentation and are therefore marked as @internal.
However, TypeScript is configured with the stripInternal compiler
option such that declarations with @internal markers are not emitted
into the .d.ts files, but this means that the reexports in the barrel
file end up referring to missing declarations.

The stripInternal option is considered internal and its documentation
states to use at your own risk (as per microsoft/TypeScript#45307).
Having the option enabled is desirable for us as it works well for
hiding class fields that are marked @internal, which is an effective
way to hide members from the .d.ts file. As a workaround for the issue
with top-level symbols, the declarations with @internal markers are
moved to dedicated files for which no public api guard is setup,
therefore allowing their @internal markers to be dropped.

Fixes #43097

@JoostK JoostK added area: packaging Issues related to Angular's creation of npm packages target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler labels Sep 21, 2021
@ngbot ngbot bot modified the milestone: Backlog Sep 21, 2021
@google-cla google-cla bot added the cla: yes label Sep 21, 2021
@JoostK JoostK marked this pull request as ready for review September 22, 2021 15:29
@JoostK JoostK added the action: review The PR is still awaiting reviews from at least one requested reviewer label Sep 22, 2021
@pullapprove pullapprove bot requested a review from atscott September 22, 2021 15:30
@JoostK JoostK added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Sep 23, 2021
@huineng
Copy link

huineng commented Oct 1, 2021

any progress on this ?

@JoostK
Copy link
Member Author

JoostK commented Oct 1, 2021

@huineng This introduces a conflict when syncing into Google and we're currently pushing towards getting v13 finalized, so this got stalled.

@JoostK JoostK added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 1, 2021
@alan-agius4 alan-agius4 removed the action: merge The PR is ready for merge by the caretaker label Oct 13, 2021
…ernal markers

The `ErrorCode` enum in the `error_code.ts` file is governed by public
api guards but the other top-level exports from that file are exempt
from public api documentation and are therefore marked as `@internal`.
However, TypeScript is configured with the `stripInternal` compiler
option such that declarations with `@internal` markers are not emitted
into the `.d.ts` files, but this means that the reexports in the barrel
file end up referring to missing declarations.

The `stripInternal` option is considered internal and its documentation
states to use at your own risk (as per microsoft/TypeScript#45307).
Having the option enabled is desirable for us as it works well for
hiding class fields that are marked `@internal`, which is an effective
way to hide members from the .d.ts file. As a workaround for the issue
with top-level symbols, the declarations with `@internal` markers are
moved to dedicated files for which no public api guard is setup,
therefore allowing their `@internal` markers to be dropped.

Fixes angular#43097
…arate file

Prior refactorings caused unexpected g3 sync issues due to a patch that
changes the error documentation URL. This commit moves the base url into
a separate file to make this more apparent.
@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 26, 2021
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 26, 2021
@AndrewKushnir
Copy link
Contributor

Note to the Caretaker: syncing this PR would require extra steps. Please reach out to @AndrewKushnir to get more info.

@jessicajaniuk
Copy link
Contributor

This PR was merged into the repository by commit 28a40f3.

jessicajaniuk pushed a commit that referenced this pull request Oct 26, 2021
…ernal markers (#43527)

The `ErrorCode` enum in the `error_code.ts` file is governed by public
api guards but the other top-level exports from that file are exempt
from public api documentation and are therefore marked as `@internal`.
However, TypeScript is configured with the `stripInternal` compiler
option such that declarations with `@internal` markers are not emitted
into the `.d.ts` files, but this means that the reexports in the barrel
file end up referring to missing declarations.

The `stripInternal` option is considered internal and its documentation
states to use at your own risk (as per microsoft/TypeScript#45307).
Having the option enabled is desirable for us as it works well for
hiding class fields that are marked `@internal`, which is an effective
way to hide members from the .d.ts file. As a workaround for the issue
with top-level symbols, the declarations with `@internal` markers are
moved to dedicated files for which no public api guard is setup,
therefore allowing their `@internal` markers to be dropped.

Fixes #43097

PR Close #43527
jessicajaniuk pushed a commit that referenced this pull request Oct 26, 2021
…arate file (#43527)

Prior refactorings caused unexpected g3 sync issues due to a patch that
changes the error documentation URL. This commit moves the base url into
a separate file to make this more apparent.

PR Close #43527
jessicajaniuk pushed a commit that referenced this pull request Oct 26, 2021
…ernal markers (#43527)

The `ErrorCode` enum in the `error_code.ts` file is governed by public
api guards but the other top-level exports from that file are exempt
from public api documentation and are therefore marked as `@internal`.
However, TypeScript is configured with the `stripInternal` compiler
option such that declarations with `@internal` markers are not emitted
into the `.d.ts` files, but this means that the reexports in the barrel
file end up referring to missing declarations.

The `stripInternal` option is considered internal and its documentation
states to use at your own risk (as per microsoft/TypeScript#45307).
Having the option enabled is desirable for us as it works well for
hiding class fields that are marked `@internal`, which is an effective
way to hide members from the .d.ts file. As a workaround for the issue
with top-level symbols, the declarations with `@internal` markers are
moved to dedicated files for which no public api guard is setup,
therefore allowing their `@internal` markers to be dropped.

Fixes #43097

PR Close #43527
jessicajaniuk pushed a commit that referenced this pull request Oct 26, 2021
…arate file (#43527)

Prior refactorings caused unexpected g3 sync issues due to a patch that
changes the error documentation URL. This commit moves the base url into
a separate file to make this more apparent.

PR Close #43527
jessicajaniuk pushed a commit that referenced this pull request Oct 26, 2021
…arate file (#43527)

Prior refactorings caused unexpected g3 sync issues due to a patch that
changes the error documentation URL. This commit moves the base url into
a separate file to make this more apparent.

PR Close #43527
jessicajaniuk pushed a commit to jessicajaniuk/angular that referenced this pull request Oct 26, 2021
@JoostK JoostK mentioned this pull request Oct 27, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 26, 2021
Serginho pushed a commit to TuLotero/angular that referenced this pull request Jan 20, 2022
…ernal markers (angular#43527)

The `ErrorCode` enum in the `error_code.ts` file is governed by public
api guards but the other top-level exports from that file are exempt
from public api documentation and are therefore marked as `@internal`.
However, TypeScript is configured with the `stripInternal` compiler
option such that declarations with `@internal` markers are not emitted
into the `.d.ts` files, but this means that the reexports in the barrel
file end up referring to missing declarations.

The `stripInternal` option is considered internal and its documentation
states to use at your own risk (as per microsoft/TypeScript#45307).
Having the option enabled is desirable for us as it works well for
hiding class fields that are marked `@internal`, which is an effective
way to hide members from the .d.ts file. As a workaround for the issue
with top-level symbols, the declarations with `@internal` markers are
moved to dedicated files for which no public api guard is setup,
therefore allowing their `@internal` markers to be dropped.

Fixes angular#43097

PR Close angular#43527
Serginho pushed a commit to TuLotero/angular that referenced this pull request Jan 20, 2022
…arate file (angular#43527)

Prior refactorings caused unexpected g3 sync issues due to a patch that
changes the error documentation URL. This commit moves the base url into
a separate file to make this more apparent.

PR Close angular#43527
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: packaging Issues related to Angular's creation of npm packages cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing types in compiler-cli
6 participants