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

[4.x] Don't allow injecting classes without @Injectable #13820

Closed
vicb opened this Issue Jan 7, 2017 · 3 comments

Comments

Projects
None yet
4 participants
@vicb
Contributor

vicb commented Jan 7, 2017

  • Injectable or any sub classes,
  • Should be deprecated in 4.x, remove in the next major release,
  • As of now it is possible when the ctor has no arguments or if any other decorator is applied.
@trotyl

This comment has been minimized.

Show comment
Hide comment
@trotyl

trotyl Jan 17, 2017

Contributor

I have a question of why making useClass so different?

Would @Injectable() also be mandatory for useFactory when decorator for function expression implementation being ready?
If so, is it a good way to use the same decorator reference to supporting two different decorator types?
If not, why everything using useFactory/useValue... is injectable by default while what in useClass needs explicitly declared to be injectable?

Contributor

trotyl commented Jan 17, 2017

I have a question of why making useClass so different?

Would @Injectable() also be mandatory for useFactory when decorator for function expression implementation being ready?
If so, is it a good way to use the same decorator reference to supporting two different decorator types?
If not, why everything using useFactory/useValue... is injectable by default while what in useClass needs explicitly declared to be injectable?

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 4, 2017

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 5, 2017

feat(compiler): shouldn't allow injecting classes without @Injectable
BREAKING CHANGE:

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeClass {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 5, 2017

feat(compiler): shouldn't allow injecting classes without @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable for contructors without args or if any other decorator was
applied. After this commit @Injectable is mandatory for all providers
provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeClass {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

feat(compiler): wip

feat(compiler): wip

feat(compiler): wip

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 5, 2017

feat(compiler): should allow injecting classes with @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable for contructors without args or if any other decorator was
applied. After this commit @Injectable is mandatory for all providers
provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

feat(compiler): wip

feat(compiler): wip

feat(compiler): wip

feat(compiler): wip

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 5, 2017

feat(compiler): should only inject services annotated with @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable when a service had contructor without args or if any other decorator was
applied. After this commit @Injectable is mandatory for all services
provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 5, 2017

feat(compiler): should only inject services annotated with @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable when a service had contructor without args or if any other decorator was
applied. After this commit @Injectable is mandatory for all services
provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 5, 2017

feat(compiler): should only inject services annotated with @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable when a service had contructor without args or if any other
decorator was applied. After this commit @Injectable is mandatory for
all services provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 12, 2017

feat(compiler): should only inject services annotated with @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable when a service had contructor without args or if any other
decorator was applied. After this commit @Injectable is mandatory for
all services provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820

DzmitryShylovich added a commit to DzmitryShylovich/angular that referenced this issue Mar 12, 2017

feat(compiler): should only inject services annotated with @Injectable
BREAKING CHANGE: Before this commit it was not nessesary to add
@Injectable when a service had contructor without args or if any other
decorator was applied. After this commit @Injectable is mandatory for
all services provided using useClass.

Before:

class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

After:

@Injectable()
class SomeService {}

@component({selector: 'my-app', template: ''})
class AppComponent {
  constructor(service: SomeService) {}
}

Closes #12467
Closes #13820
@Toxicable

This comment has been minimized.

Show comment
Hide comment
@Toxicable

Toxicable Apr 8, 2017

Contributor

Was this not fixed via b23ad39 ?

Contributor

Toxicable commented Apr 8, 2017

Was this not fixed via b23ad39 ?

@chuckjaz

This comment has been minimized.

Show comment
Hide comment
@chuckjaz

chuckjaz Apr 13, 2017

Member

Closed by #14931

Member

chuckjaz commented Apr 13, 2017

Closed by #14931

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