fix(core): hide implementation details of ExperimentalPendingTasks#55516
fix(core): hide implementation details of ExperimentalPendingTasks#55516pkozlowski-opensource wants to merge 1 commit intoangular:mainfrom
Conversation
The ExperimentalPendingTasks service was accidently exposing one of its internal fields as a public one. This commit fixes this by marking the field in question as private.
thePunderWoman
left a comment
There was a problem hiding this comment.
LGTM
reviewed-for: fw-core, public-api
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken |
|
caretaker note: G3 failures are unrelated (deflaked presubmit) |
|
This PR was merged into the repository by commit 90389ad. |
| }) | ||
| export class ExperimentalPendingTasks { | ||
| internalPendingTasks = inject(PendingTasks); | ||
| private internalPendingTasks = inject(PendingTasks); |
There was a problem hiding this comment.
Any reason not to do
#internalPendingTasks = inject(PendingTasks);There was a problem hiding this comment.
Private props are not supported by the internal google toolings (yet).
There was a problem hiding this comment.
It's not just lack of support, but there are also real downsides to using them. For reference: https://google.github.io/styleguide/tsguide.html#class-members
Private identifiers cause substantial emit size and performance regressions when down-leveled by TypeScript, and are unsupported before ES2015. They can only be downleveled to ES2015, not lower. At the same time, they do not offer substantial benefits when static type checking is used to enforce visibility.
There was a problem hiding this comment.
We are using it with ES2022 target I hope there are no performance issues with ES2022 target, as TS won't downlevel it.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The ExperimentalPendingTasks service was accidentally exposing one of its internal fields as a public one. This commit fixes this by marking the field in question as private.