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

Ensure packages can be built with TypeScript strict flag #30993

Closed

Conversation

@devversion
Copy link
Member

commented Jun 12, 2019

Implemented as part of FW-1265. See individual commits for description of changes.

@googlebot googlebot added the cla: yes label Jun 12, 2019

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch 2 times, most recently from 4a1d923 to f93e879 Jun 12, 2019

@ngbot ngbot bot modified the milestone: needsTriage Jun 12, 2019

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch from f93e879 to fd10216 Jun 12, 2019

@devversion devversion marked this pull request as ready for review Jun 12, 2019

@devversion devversion requested review from angular/fw-compiler as code owners Jun 12, 2019

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch from fd10216 to f3a7180 Jun 12, 2019

@devversion devversion requested a review from angular/fw-public-api as a code owner Jun 12, 2019

@gkalpak
Copy link
Member

left a comment

🎉

packages/core/src/util/decorators.ts Outdated Show resolved Hide resolved
packages/core/src/render3/di.ts Show resolved Hide resolved
packages/core/src/debug/debug_node.ts Outdated Show resolved Hide resolved
packages/compiler/src/render3/view/template.ts Outdated Show resolved Hide resolved
packages/tsconfig-build-strict.json Outdated Show resolved Hide resolved
packages/BUILD.bazel Outdated Show resolved Hide resolved

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch from f3a7180 to 0ef9400 Jun 12, 2019

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch from 0ef9400 to 7a51f10 Jun 13, 2019

@devversion devversion requested review from angular/fw-forms as code owners Jun 13, 2019

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch from 7a51f10 to c30cdbf Jun 13, 2019

@devversion devversion requested a review from IgorMinar as a code owner Jun 13, 2019

devversion added some commits Jun 13, 2019

refactor(router): compatibility with typescript strict flag
As part of FW-1265, the `@angular/router` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
refactor(forms): ensure compatibility with typescript strict flag
As part of FW-1265, the `@angular/forms` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
refactor(service-worker): ensure compatibility with typescript strict…
… flag

As part of FW-1265, the `@angular/service-worker` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
refactor: fix typescript strict flag failures in all tests
Fixes all TypeScript failures caused by enabling the `--strict`
flag for test source files. We also want to enable the strict
options for tests as the strictness enforcement improves the
overall codehealth, unveiled common issues and additionally it
allows us to enable `strict` in the `tsconfig.json` that is picked
up by IDE's.
fix(platform-browser): debug element query predicates not compatible …
…with strictFunctionTypes

Currently developers can use the `By` class to construct common
`DebugElement` query predicates. e.g. `By.directive(MyDirective)`.

The `directive()` and `all()` predicates are currently returning
a predicate that works for `DebugElement` nodes. This return type
is too strict since the predicate is not specific to `DebugElement`
instances and can also apply to `DebugNode` instances.

Meaning that developers are currently able to use the `directive()`
predicate when using `queryAllNodes()`. This is a common practice
but will break when the project is compiled with TypeScript's
`--strictFunctionTypes` flag as the `DebugElement` predicate type
is not assignable to predicates for `DebugNode`. In order to make
these predicates usable with `--strictFuntionTypes` enabled, we
adjust the predicate type to reflect what is actually needed for
evaluation of the predicate.
refactor: ensure zone.js can be built with typescript strict flag
As part of FW-1265, the `zone.js` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

@devversion devversion force-pushed the devversion:refactor/core-wip-strict-flag branch from 25de548 to f4917c3 Jul 18, 2019

*
* @usageNotes
* ### Example
*
* {@example platform-browser/dom/debug/ts/by/by.ts region='by_directive'}
*/
static directive(type: Type<any>): Predicate<DebugElement> {
return (debugElement) => debugElement.providerTokens !.indexOf(type) !== -1;
static directive(type: Type<any>): Predicate<DebugNode> {

This comment has been minimized.

Copy link
@devversion

devversion Jul 18, 2019

Author Member

These return type changes can seem like a breaking change, but Predicate<DebugNode> is also assignable to Predicate<DebugElement>, so it should not be breaking.

It just fixes the type so that debugEl.query(By.directive(..)) also works for consumer applications using strictFunctionTypes. Here is a playground demo: see here

@JiaLiPassion
Copy link
Contributor

left a comment

Thanks!

@IgorMinar
Copy link
Member

left a comment

thanks Paul!!

@mhevery

This comment has been minimized.

@mhevery mhevery closed this in 2200884 Jul 18, 2019

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(compiler): ensure compatibility with typescript strict flag (#…
…30993)

As part of FW-1265, the `@angular/compiler` package is made compatible
with the TypeScript `--strict` flag. This already unveiled a few bugs,
so the strictness flag seems to help with increasing the overall code health.

Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor: fix remaining typescript strict flag failures (#30993)
Fixes the remaining TypeScript --strict flag failures for source
files which are not part of any specific release package.

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

fix(ivy): incorrect type definition for ɵɵdefineComponent (#30993)
Currently the `ɵɵdefineComponent` method has incorrect type
definitions the `directives` and `pipes` metadata property.

The incorrect types allow developers to pass in already instantiated
`DirectiveDef` or `ComponentDef` objects. This can cause unexpected
failures because the definition internally only expects `Type` objects
and now incorrectly tries to read the `ngDirectiveDef` or `ngComponentDef`
of existing definitions.

This issue has been unveiled by enabling the strict function parameter
types flag, where the directive definitions are determined from each array
element in the `directives` or `pipes` property (which can throw).

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

fix(ivy): semantic module check incorrectly handles nested arrays (#3…
…0993)

In View Engine, developers can pass bootstrap and entry components
as nested arrays. e.g.

```ts
export const MyOtherEntryComponents = [A, B, C]

@NgModule({
  entryComponents: [MyComp, MyOtherEntryComponents]
})
```

Currently using nested arrays for these properties causes
unexpected errors to be reported in Ivy since the semantic
NgModule checks aren't properly recursing into the nested
entry/bootstrap components. This issue has been unveiled by
enabling the strict function parameter checks.

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(common): ensure compatibility with typescript strict flag (#…
…30993)

As part of FW-1265, the `@angular/common` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(upgrade): ensure compatibility with typescript strict flag (#…
…30993)

As part of FW-1265, the `@angular/upgrade` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(platform-browser): compatibility with typescript strict flag (
…#30993)

As part of FW-1265, the `@angular/platform-browser` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(router): compatibility with typescript strict flag (#30993)
As part of FW-1265, the `@angular/router` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(forms): ensure compatibility with typescript strict flag (#3…
…0993)

As part of FW-1265, the `@angular/forms` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor(service-worker): ensure compatibility with typescript strict…
… flag (#30993)

As part of FW-1265, the `@angular/service-worker` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor: fix typescript strict flag failures in all tests (#30993)
Fixes all TypeScript failures caused by enabling the `--strict`
flag for test source files. We also want to enable the strict
options for tests as the strictness enforcement improves the
overall codehealth, unveiled common issues and additionally it
allows us to enable `strict` in the `tsconfig.json` that is picked
up by IDE's.

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

fix(platform-browser): debug element query predicates not compatible …
…with strictFunctionTypes (#30993)

Currently developers can use the `By` class to construct common
`DebugElement` query predicates. e.g. `By.directive(MyDirective)`.

The `directive()` and `all()` predicates are currently returning
a predicate that works for `DebugElement` nodes. This return type
is too strict since the predicate is not specific to `DebugElement`
instances and can also apply to `DebugNode` instances.

Meaning that developers are currently able to use the `directive()`
predicate when using `queryAllNodes()`. This is a common practice
but will break when the project is compiled with TypeScript's
`--strictFunctionTypes` flag as the `DebugElement` predicate type
is not assignable to predicates for `DebugNode`. In order to make
these predicates usable with `--strictFuntionTypes` enabled, we
adjust the predicate type to reflect what is actually needed for
evaluation of the predicate.

PR Close #30993

mhevery added a commit that referenced this pull request Jul 18, 2019

refactor: ensure zone.js can be built with typescript strict flag (#3…
…0993)

As part of FW-1265, the `zone.js` package is made compatible
with the TypeScript `--strict` flag. Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)

PR Close #30993

devversion added a commit to devversion/angular that referenced this pull request Aug 2, 2019

build: ensure schematics are built with typescript strict flag
Follow-up to angular#30993 where we build all Angular packages with
the TypeScript `--strict` flag. The flag improves overall code
health and also helps us catch issues easier.

devversion added a commit to devversion/angular that referenced this pull request Aug 13, 2019

build: ensure schematics are built with typescript strict flag (angul…
…ar#31967)

Follow-up to angular#30993 where we build all Angular packages with
the TypeScript `--strict` flag. The flag improves overall code
health and also helps us catch issues easier.

PR Close angular#31967

kara added a commit that referenced this pull request Aug 13, 2019

build: ensure schematics are built with typescript strict flag (#31967)
Follow-up to #30993 where we build all Angular packages with
the TypeScript `--strict` flag. The flag improves overall code
health and also helps us catch issues easier.

PR Close #31967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.