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

Better IoC support #21531

Closed
tinganho opened this issue Feb 1, 2018 · 12 comments
Closed

Better IoC support #21531

tinganho opened this issue Feb 1, 2018 · 12 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision

Comments

@tinganho
Copy link
Contributor

tinganho commented Feb 1, 2018

It would be nice if TS could be more IoC friendly. One of the main pains I have with it, is that it doesn't know an alternative constructor:

import { Component } from '@angular/component';

@Component({})
export class MyComponent {
    private a: string; // Error it is never initialized.
    ngOnInit() {
        this.a =  1
    }
}

Here we annotate a secondary constructor on IComponent:

interface IComponent {
    constructor ngOnInit?();
}

And then we instructs the class to implement it, note the use of implements keyword:

function Component(options: any) {
    return function<T extends Function>(_class: T implements IComponent): void {
         /*...*/
    }
}

And then, all people can be happy:

@Component({})
export class MyComponent {
    private a: string; // No error
    ngOnInit() {
        this.a =  1
    }
}

For property decorators, we currently face this problem:

@Component({})
export class MyComponent {
    @Input() a: string; // Error no init.
}

Here we introduce the initializes keyword:

function Component(options: any) {
    return function<T extends Function>(_class: T implements IComponent initializes @Input): void {
         /*...*/
    }
}

Other frameworks are built the other way around, instead of another disjoint function or class providing the construction of the class, a base class provides it. So we could theoretically write our component like below(if we disregard the class annotations ):

export class MyComponent extends BaseComponent {
    @Input() a: string;
    private b: string;
    ngOnInit() {
        this.b = ''; 
    }
}

And we should be able to achieve the same thing using a base class:

export class BaseComponent initializes @Input {
    constructor ngOnInit?(): void
}
@j-oliveras
Copy link
Contributor

This is a breaking change into 2.7 version if you use strict mode. To disable it, set strictPropertyInitialization to false.

@tinganho
Copy link
Contributor Author

tinganho commented Feb 1, 2018

Yes, I'm aware of that. This is more of a proposal to work with the flag on.

@AlCalzone
Copy link
Contributor

AlCalzone commented Feb 1, 2018

How about this?

import { Component } from '@angular/component';

@Component({})
export class MyComponent {
    private a!: string; // notice the exclamation mark!
    ngOnInit() {
        this.a =  1
    }
}

or am I missing the point?

@tinganho
Copy link
Contributor Author

tinganho commented Feb 1, 2018

I didn't mentioned it above. But the proposal is so you don't have to use the definite assignment operator ! or the --strictPropertyInitialization flag. A consumer of one framework trust the framework and so should the compiler. Right now, there is noway of encoding that in the language, and that is what this proposal is about.

The compiler should trust that the framework will construct and initialize variables if I configured them to do so. Right now, the compiler doesn't trust the framework and I shouldn't have to opt-out of strict checking because it can't.

I hope this clarifies things

@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2018

Similar discussion in #20740.

I do not think we would add a new syntax just to disable checking of uninitialized properties.. this is an opt-in feature, and users can chose to enable it (or disable it under --strict), we also have a per-property opt-out with the definite assignment operator.

@mhegazy mhegazy added the Declined The issue was declined as something which matches the TypeScript vision label Feb 1, 2018
@tinganho
Copy link
Contributor Author

tinganho commented Feb 1, 2018

I've read before that you where considering a syntax for it?

#20740 (comment)
And one more comment that I cannot find at the moment.

I get your point, but I think people(including me) want to opt-in using --strict, but without using the ! hack. It feels like ! is just a temporary solution until the compiler can be smarter. At least given, that the compiler becomes smart enough a ! shouldn't be necessary.

Now, if I upgrade my project to the new version of TS, I have to either opt-out with --strictPropertyInitialization=false. Or put ! in maybe in half of my code base — and I'm not exagerating. And then when you decide to make the compiler smart enough, we have to remove all the !. There is a solution that can prevent both fallbacks and that's what I presented here.

Feels a bit that proposals being declined without being read or thought nor letting other have time to leave their comments.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2018

It feels like ! is just a temporary solution until the compiler can be smarter.

no it is not a hack, and it is not temporary.. they are cases that the compiler does not know. they are cases, where the init function is called multiple times, and there are cases, where undefined is used explicitly to mean uninitialized in delayed initialization scenarios.. we have these in our code base, and adding this operator came from an experiment adding --strictNullChecks in our own code base.

Now, if I upgrade my project to the new version of TS, I have to either opt-out with --strictPropertyInitialization=false. Or put ! in maybe in half of my code base — and I'm not exaggerating.

That is why you have two means of disabling this check.. you need to decide what is more economical for you.. either you disable it across the whole code base, or you opt-out specific classes.

Feels a bit that proposals being declined without being read or thought or not letting other have time to comment.

that is not true. we have been discussing this issue for a few month now.. so I can claim that i have thought about many of these issues already, and considered them.

@tinganho
Copy link
Contributor Author

tinganho commented Feb 1, 2018

that is not true. we have been discussing this issue for a few month now.. so I can claim that i have thought about many of these issues already, and considered them.

I appreciate all the hard work you done and I know the TS team have put a lot of effort on this. I was meaning more that any proposal, in this case that maybe is an improvement, are declined very fast. I put some hours on my free time writing this proposal as well. Because it was declined so instantly, I can only interpret it as you don't want to listen to any or the proposal was bad.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2018

The issue is open, and i am replying, and others are reading.. so no one is opposed to listening... We have discussed similar proposals in the past, that is all.

@tinganho
Copy link
Contributor Author

tinganho commented Feb 1, 2018

And the decline label?

@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2018

We have discussed similar proposals in the past and decided not to do them. hence the declined label.

@typescript-bot
Copy link
Collaborator

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision
Projects
None yet
Development

No branches or pull requests

5 participants