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

TypeScript 2.7 support #21571

Closed
IgorMinar opened this Issue Jan 16, 2018 · 33 comments

Comments

Projects
None yet
@IgorMinar
Member

IgorMinar commented Jan 16, 2018

TypeScript 2.7 is coming out in January. We need to get it supported within the v6 timeframe.

  • #22097 ensure that we can compile our codebase with 2.7
  • make the compiler compatible with 2.7
  • retain support for TypeScript 2.6 to make g3 upgrade easier

@IgorMinar IgorMinar added this to the v6.0 milestone Jan 16, 2018

@cyrilletuzi

This comment has been minimized.

Contributor

cyrilletuzi commented Jan 23, 2018

Until now, when Angular supported a new version of TypeScript, it supported it in strict mode.

But in TS 2.7, --strictPropertyInitialization causes a lot of mess in Angular code, as reported in Microsoft/TypeScript#20740. Solutions have been discussed, but nothing seems to be included in the 2.7 release.

What's the plans about that ?

@IgorMinar

This comment has been minimized.

Member

IgorMinar commented Jan 23, 2018

@IgorMinar

This comment has been minimized.

Member

IgorMinar commented Feb 2, 2018

I discussed the --strictPropertyInitialization flag with @mhevery, considered various options, and came to a conclusion that the flag does the right thing and any kind of special treatment for Angular's @Input fields would be compromising the type model.

Our recommendation is to either don't use the flag, or the better option is to initialize the inputs to a default value or null (which requires | null type union).

@nicolashenry

This comment has been minimized.

nicolashenry commented Feb 2, 2018

@IgorMinar

Why can not inputs be defined in the constructor? Like this for example :

@Component({ selector: 'my-component' })
class MyComponent {
    constructor(@Input() private myInput: string) {}
}

This would enable a proper use of --strictPropertyInitialization. I understood from documentation about ngOnInit that this may be needed for change detection but I do not understand why change detection need to be started before all inputs are available.

@mhevery

This comment has been minimized.

Member

mhevery commented Feb 2, 2018

Inputs by definition can change over time. Constructor can only receive one value. But you can use @Attribute in the constructor. Unfortunately someone needs to wire the docs: https://angular.io/api/core/Attribute :-( See spec instead: https://github.com/angular/angular/blob/master/packages/core/test/linker/integration_spec.ts#L2253-L2265 https://github.com/angular/angular/blob/master/packages/core/test/linker/integration_spec.ts#L1252-L1263

@cyrilletuzi

This comment has been minimized.

Contributor

cyrilletuzi commented Feb 2, 2018

Setting a default value is not possible for objects, which is a very common case. Setting to null does not only require to set the | null union type, it also forces to add conditions checking if it's null or not for every bit of code using that data. So it's clearly not an option, especially when it's not relevant.

The very classic architecture of Angular is : a controller component get data via a service and pass the data to a presentation component with @Input(). Check is done in the controller component with *ngIf, so we know the @Input() won't be null, and it would be crazy to add conditions checking everywhere just because of TS behavior.

So for me, it's sure it will be the option of not being strict, which is sad. I think it's really weird TS introduced a feature which is not relevant/friendly with one of the bigger project using it (and I suppose Angular won't be the only affected framework/library). As I said in the other discussion, the current logic is "let's the developer change all his/her code (even when it's good code) to manage the problems", when it should be "let's TypeScript manage its own problems".

@nicolashenry

This comment has been minimized.

nicolashenry commented Feb 2, 2018

@mhevery @Attribute seems interesting in some situations, thanks :)

Actually, I would have hoped that this (@Input on parameter property) :

@Component({ selector: 'my-component' })
class MyComponent {
    constructor(@Input('myInput') public myInput: string) {}
}

would be the equivalent to this in some way :

@Component({ selector: 'my-component' })
class MyComponent {
    @Input('myInput')
    public myInput: string;
    constructor(@Attribute('myInput') myInput: string) {
        this.myInput = myInput;
    }
}
@pgrm

This comment has been minimized.

pgrm commented Feb 5, 2018

@cyrilletuzi - I don't think that any check forces you to set the input parameters, so technically marking them as potentially undefined does make sense. I'd rather want to have a possibility within angular to say - these properties need to be set, to have the type check there. The example above, by @nicolashenry would be perfect, if it works like that.

By the way - in case I don't care for implementing a setter for myInput couldn't I directly write

@Component({ selector: 'my-component' })
class MyComponent {
    constructor(@Attribute('myInput') @Input('myInput') public myInput: string) { }
}
@mhevery

This comment has been minimized.

Member

mhevery commented Feb 5, 2018

@cyrilletuzi Nothing forces @Input() to be set. A user of the your component may simply chose not to bind to that input in which case your @Input() is not initialize and you should have checks for non-initialized case. I know we disagree, but I think this is the correct behavior.

What would be nice is to have @Input({required:true}) which would be an error if the user created your component and did not bind to that @Input, but as of yet we don't have such a feature.

@jbedard

This comment has been minimized.

Contributor

jbedard commented Feb 5, 2018

Could @Input({required:true}) not be accomplished just using ts somehow? By analyzing the @Input variable type and seeing if undefined is an acceptable type?

@cyrilletuzi

This comment has been minimized.

Contributor

cyrilletuzi commented Feb 5, 2018

What would be really nice is that @Input() is required by default, except there is a default value. Like function parameters in TypeScript and quite any language.

But with the actual behavior, yes, there should be a check, and there is one : the *ngIf on the parent controller. So again, when checks are done and the code is good, it's not normal for the developer to have to add more TS specific code just to please TypeScript.

@james-schwartzkopf

This comment has been minimized.

james-schwartzkopf commented Feb 6, 2018

Have you considered making @input, @ViewChild/@ViewChildren/@ContentChild/@ContentChilren etc injectable as Observables?

Personally I've been wanting that anyway for easier rxjs compositions, and it seems like it would solve a large percentage of the strictPropertyInitialization problem. You would still have properties that are initialized in onInit, but that would be small enough I'd probably be happy dealing with them on a case by case either adding '| null', '!', or a default as needed.

@mhevery

This comment has been minimized.

Member

mhevery commented Feb 7, 2018

chuckjaz added a commit to chuckjaz/angular that referenced this issue Feb 8, 2018

chuckjaz added a commit to chuckjaz/angular that referenced this issue Feb 8, 2018

chuckjaz added a commit to chuckjaz/angular that referenced this issue Feb 8, 2018

@sebastian-zarzycki-es

This comment has been minimized.

sebastian-zarzycki-es commented Feb 21, 2018

Any progress on this?

@skreborn

This comment has been minimized.

Contributor

skreborn commented Feb 21, 2018

@sebastian-zarzycki-es As you can see, a pull request dealing with this issue has been opened 13 days ago. It currently has merge conflicts with master, so it needs a little fixing up, but it seems to be on the way.

@sebastian-zarzycki-es

This comment has been minimized.

sebastian-zarzycki-es commented Feb 21, 2018

Yeah, I can see that. "any" was probably a bit too vague. The question really was "when, roughly, can I expect it?" :)
Also, would be interested in how the above problems were handled, eventually. Was the strict check turned off?

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

@sebastian-zarzycki-es The strict option was never turned on in Angular CLI seed, what do you mean turned off?

@sebastian-zarzycki-es

This comment has been minimized.

sebastian-zarzycki-es commented Feb 21, 2018

A comment from above - "Until now, when Angular supported a new version of TypeScript, it supported it in strict mode."

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

@sebastian-zarzycki-es Please refer to angular/devkit#218 and angular/devkit#397, this tracker is for Angular itself, not for CLI project.

@sebastian-zarzycki-es

This comment has been minimized.

sebastian-zarzycki-es commented Feb 21, 2018

I didn't bring CLI into this, you did. I've merely referred to the first cyrilletuzi's comment here. But that's really a sidequestion, I was interested in release date/estimate, mostly.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

@sebastian-zarzycki-es Since it's under the 6.0 milestone, should happen before the 6.0 stable release.

@bmayen

This comment has been minimized.

bmayen commented Feb 21, 2018

You're asking if it was "turned off". The only place this is likely to be turned on for you by Google is the CLI. So I think the assumption is that you're asking if it was turned off there... in which case the answer is that it was never turned on in the first place. Otherwise, if you're not using CLI, you can simply not enable strict checking in your own config and this won't be an issue for you.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

Also please note that these two questions are different:

  • Whether Angular .d.ts can work under new TypeScript option;
  • Whether my existing project can still work under new TypeScript option seamlessly;

The latter one is not something Angular can directly helped.

@sebastian-zarzycki-es

This comment has been minimized.

sebastian-zarzycki-es commented Feb 21, 2018

All clear now, thank you.

@fr0

This comment has been minimized.

fr0 commented Feb 21, 2018

It seems that there is a more general issue here - that developers using Angular have to wait weeks/months to upgrade to each new version of TypeScript.

Is it possible that we could get to a point where Angular immediately supports each new version of TS once it drops? I'm not sure if that's even feasible, or if it makes sense, but it seems like a worthwhile goal.

As always, thanks for the hard work.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

@fr0 That question was answered in #19587 (comment). Unlike syntax, the TypeScript compiler API would has breaking change in every minor version, so always require extra work on it.

The supporting status can be checked at https://github.com/angular/angular-cli/blob/77d2cd3c1dcd0d41eb87697f8d21809f78b586db/packages/%40angular/cli/upgrade/version.ts#L169-L175

Technically if one only use TypeScript command line tool and JIT for Angular (with their own project configuration) would be able to use latest TypeScript immediately, like we can already use latest TypeScript in Plunker w/o Angular support. But most user need more than that.

@cyrilletuzi

This comment has been minimized.

Contributor

cyrilletuzi commented Feb 21, 2018

Btw, the discussions eluded the initial question : it will be up to the developer to choose if he/she wants to work in strict mode, but will Angular itself (in its own code) support the --strictPropertyInitialization of TS 2.7 ? Because if it doesn't, working in strict mode won't be an option at all.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

but will Angular itself (in its own code) support the --strictPropertyInitialization of TS 2.7

Actually strictPropertyInitialization doesn't affect .d.ts, so makes no impact whether Angular supports it or not.

@bmayen

This comment has been minimized.

bmayen commented Feb 21, 2018

To be fair, it's still an option, but with uglier syntax :)

@cyrilletuzi

This comment has been minimized.

Contributor

cyrilletuzi commented Feb 21, 2018

@trotyl Can you explain why ? Previous stricter checks introduced by TypeScript often produced errors in compilation of an app because of errors in Angular itself.

@trotyl

This comment has been minimized.

Contributor

trotyl commented Feb 21, 2018

@cyrilletuzi The strictPropertyInitialization option cannot be applied to lib, there’s no such information (constructor implementation) exists to perform that check...

@kylecordes

This comment has been minimized.

kylecordes commented Mar 4, 2018

@trotyl I've had to make my way to that block of code in CLI before, to explain the Angular -> TypeScript supported versions. It seems to me that ideally this information would be represented canonically somewhere in Angular core (and then referred to by CLI code) as it seems to me this is inevitably a property of Angular itself that would apply even to non-CLI users. Roughly akin to the browser support information.

@emilio-martinez

This comment has been minimized.

Contributor

emilio-martinez commented Mar 5, 2018

Could not agree more, @kylecordes.

To that effect, I know that compiler feature landed somewhere in the road to 6.0 with 3ceee99. My assumption (without having tried it out myself) would be that as long as ngc complains loud enough, the CLI should just stop on its tracks if the TS version is unsupported, and developers can "opt out" to ignore via disableTypeScriptVersionCheck in the angularCompilerOptions.

IgorMinar added a commit to IgorMinar/angular that referenced this issue Mar 9, 2018

IgorMinar added a commit to IgorMinar/angular that referenced this issue Mar 9, 2018

IgorMinar added a commit to IgorMinar/angular that referenced this issue Mar 10, 2018

IgorMinar added a commit to IgorMinar/angular that referenced this issue Mar 10, 2018

IgorMinar added a commit to IgorMinar/angular that referenced this issue Mar 10, 2018

IgorMinar added a commit to IgorMinar/angular that referenced this issue Mar 10, 2018

@kara kara closed this in 8449eb8 Mar 12, 2018

leo6104 added a commit to leo6104/angular that referenced this issue Mar 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment