presubmit

This increases bundle size for no actual benefit as far as I can see? It seems like a valid use of this TypeScript functionality.

Either that, or it should be | undefined and then the code adjusted for that.

@alfaproject Changing type to Function | undefined would require more code because the function type should be narrowed from Function | undefined to Function using if statements. Before:

this.resolve();

After:

if (this.resolve) {
  this.resolve();
}

The same applies to this.reject. Removing non-null assertion operator might prevent runtime errors in the future. There is a TSLint rule that describes the cons of this operator: https://palantir.github.io/tslint/rules/no-non-null-assertion/
The similar commits were already merged in the past: 1b0580a#diff-6a59e357bd47ab780ce7fcf88e9dfc50

@kubk please rebase.

@kubk I mean generated code. Check the down-levelled version in ES5 (it also uses more memory, btw)

Not sure what's the problem in keeping the non-null assertion operator to keep it lean. It exists for a reason. I would just remove the TODO comments or replace them with a more descriptive comment of why it's ok to have it, but I'm not a maintainer d:

Hi @kubk, thanks for rebasing your PR. It looks like the integration_test is failing with payload size checks (see https://circleci.com/gh/angular/angular/648028#tests/containers/2). Could you please update the file mentioned in the error message? Thank you.

@AndrewKushnir Done. Do you know how to fix the rest of the issues?

Hi @kubk, it looks like CI errors are flakes, I've restarted failing CI jobs.

I've also added @kara as a reviewer for this PR, since it increases payload size limits.

Thank you.

Closing in favor of #39730

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.}