Skip to content
This repository was archived by the owner on Dec 18, 2024. It is now read-only.

build: enable strictTemplates, strictBindCallApply, and strictFunctionTypes #688

Merged
merged 1 commit into from
Dec 16, 2019

Conversation

Splaktar
Copy link
Contributor

@Splaktar Splaktar commented Dec 13, 2019

  • enable Ivy's strict Template Type Checking
  • fix related build errors
  • complete this._destroyed in ngOnDestroy()
  • exclude dist/ from linting

strictPropertyInitialization is a bit more problematic, so it will be handled in a separate PR.

@jelbourn
Copy link
Member

I'm also not really concerned w/ turning on strictPropertyInitialization, which doesn't play super well w/ Angular in general

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -71,8 +70,12 @@ export class ThemePicker implements OnInit, OnDestroy {

ngOnInit() {
this._queryParamSubscription = this._activatedRoute.queryParamMap
.pipe(map((params: ParamMap) => params.get('theme')), filter(Boolean))
.subscribe((themeName: string) => this.installTheme(themeName));
.pipe(map((params: ParamMap) => params.get('theme')))
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 the previous approach was valid, you just needed to do themeName! since it's guaranteed to be defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

    this._queryParamSubscription = this._activatedRoute.queryParamMap
    .pipe(map((params: ParamMap) => params.get('theme')), filter(Boolean))
    .subscribe((themeName: string) => this.installTheme(themeName!));

Results in:

ERROR in src/app/shared/theme-picker/theme-picker.ts:74:16 - error TS2769: No overload matches this call.
  Overload 1 of 5, '(observer?: NextObserver<unknown> | ErrorObserver<unknown> | CompletionObserver<unknown> | undefined): Subscription', gave the following error.
    Argument of type '(themeName: string) => void' is not assignable to parameter of type 'NextObserver<unknown> | ErrorObserver<unknown> | CompletionObserver<unknown> | undefined'.
      Property 'complete' is missing in type '(themeName: string) => void' but required in type 'CompletionObserver<unknown>'.
  Overload 2 of 5, '(next?: ((value: unknown) => void) | undefined, error?: ((error: any) => void) | undefined, complete?: (() => void) | undefined): Subscription', gave the following error.
    Argument of type '(themeName: string) => void' is not assignable to parameter of type '(value: unknown) => void'.
      Types of parameters 'themeName' and 'value' are incompatible.
        Type 'unknown' is not assignable to type 'string'.

74     .subscribe((themeName: string) => this.installTheme(themeName!));
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  node_modules/rxjs/internal/types.d.ts:64:5
    64     complete: () => void;
           ~~~~~~~~
    'complete' is declared here.

This compiles, but is not great...

.subscribe((themeName: any) => this.installTheme(themeName));

…nTypes

- fix related build errors
- complete `this._destroyed` in `ngOnDestroy()`
- exclude `dist/` from linting
@Splaktar Splaktar force-pushed the strict-template-type-checking branch from c3509a9 to 8c57c56 Compare December 16, 2019 05:31
@jelbourn jelbourn merged commit 32d353d into master Dec 16, 2019
@Splaktar Splaktar deleted the strict-template-type-checking branch December 16, 2019 22:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants