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.

Sign up for GitHub

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

Angular: Make CLI templates compatible with TS strict mode #12081

Merged
merged 2 commits into from
Aug 17, 2020
Merged

Angular: Make CLI templates compatible with TS strict mode #12081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61c640b
e2e: use TS strict mode in Angular e2e tests
gaetanmaisse Aug 17, 2020
37318e9
fix(cli): make Angular templates compatible with TS strict mode
gaetanmaisse Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/cli/src/frameworks/angular/button.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ export default class ButtonComponent {
* Is this the principal call to action on the page?
*/
@Input()
primary: boolean;
primary = false;

/**
* What background color to use
*/
@Input()
backgroundColor: string;
backgroundColor?: string;

/**
* How large should the button be?
Expand All @@ -37,7 +37,7 @@ export default class ButtonComponent {
* @required
*/
@Input()
label: string;
label = 'Button';
Copy link
Member

@shilman shilman Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Why can't this just be a required string? I don't know if it makes sense to have a default value?

Copy link
Member

Choose a reason for hiding this comment

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

Also, welcome back!! 🎉

Copy link
Member Author

@gaetanmaisse gaetanmaisse Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@shilman you mean optional? I was going to make it optional but the @required 2 lines above and it scared me 😱 😄 .

If we want to keep it as "string" we need to provide a default value, otherwise, it's a string | undefined

Copy link
Member

@shilman shilman Aug 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Why can't it be required and NOT have a default value?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because of strictPropertyInitialization (enabled in strict mode): https://devblogs.microsoft.com/typescript/announcing-typescript-2-7/#stricter-class-property-checks

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for that. TypeScript 🤦

Copy link

Choose a reason for hiding this comment

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

Well, this actually makes sense. The Angular compiler does not force you to set required Inputs in your template. Thus, even if you claim that an Input is required, at runtime it might be undefined if you don't assign a default value.

On the other hand, even if the Angular compiler enforced this, how would the framework-agnostic Typescript compiler know this?

So, this problem arises from the way of how components are built upon Typescript classes.


/**
* Optional click handler
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/src/frameworks/angular/header.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { Component, Input, Output, EventEmitter } from '@angular/core';
})
export default class HeaderComponent {
@Input()
user = null;
user: unknown = null;

@Output()
onLogin = new EventEmitter<Event>();
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/src/frameworks/angular/page.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import { Component, Input, Output, EventEmitter } from '@angular/core';
})
export default class PageComponent {
@Input()
user = null;
user: unknown = null;

@Output()
onLogin = new EventEmitter<Event>();
Expand Down
2 changes: 1 addition & 1 deletion scripts/run-e2e-config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const baseAngular: Parameters = {
version: 'latest',
generator: [
`yarn add @angular/cli@{{version}} --no-lockfile --non-interactive --silent --no-progress`,
`npx ng new {{name}}-{{version}} --routing=true --minimal=true --style=scss --skipInstall=true`,
`npx ng new {{name}}-{{version}} --routing=true --minimal=true --style=scss --skipInstall=true --strict`,
].join(' && '),
additionalDeps: ['react', 'react-dom'],
};
Expand Down