Repository navigation
Conversation
eea2e36 to
19089f5
Compare
There was a problem hiding this comment.
Because the .d.ts file now has PACKAGE_ROOT_URL from any to OpaqueToken. So the import is needed to make .d.ts happy, even thought OpaqueToken is not used in this file directly (only through inference)
There was a problem hiding this comment.
should we use _, __, ___, ... for parameter names at this location ?
There was a problem hiding this comment.
I think names should have a meaning. Would like to keep as is.
There was a problem hiding this comment.
Array<> => []
should be a tslint rule probably
There was a problem hiding this comment.
But that would be (Validator|Function)[] which to me looks strange (@chuckjaz agrees).
There was a problem hiding this comment.
because jsonBackend is declared as MockBackend. injector.get(JSONPBackend) returns type JSONPBackend which is a subtype of MockBackend hence cast is required.
There was a problem hiding this comment.
Consider changing the type to OpaqueToken<Array<() => void>>.
There was a problem hiding this comment.
Consider changing the type to OpaqueToken<Array<(compRef: ComponentRef<any>) => void>>.
IgorMinar
left a comment
There was a problem hiding this comment.
As raised by others this looks like a bigger breaking change that would have a significant impact on the ecosystem. Our promise was that we are not going to break existing components on npm.
Can we evaluate and document the impact of this change on the ecosystem (existing 3rd party components, applications, tooling, etc) before we merge this in?
4d95a76 to
dce3247
Compare
|
@IgorMinar / @vicb Could you have a look? This should be a lot less breaking now. |
There was a problem hiding this comment.
Why not Token (or InjectionProvider, ...)
There was a problem hiding this comment.
discussed in person
There was a problem hiding this comment.
rename opaqueToken ? (grep the whole project)
There was a problem hiding this comment.
this is for old OpaqueToken should be left as is.
IgorMinar
left a comment
There was a problem hiding this comment.
please fix the docs. otherwise LGTM!
There was a problem hiding this comment.
can you explain why? (can't carry type info)
There was a problem hiding this comment.
please create this as a standalone example and then use @example to include it here. we've done this for other apis already. Please no more new inline code snippets.
12f97ff to
66a6f96
Compare
- Introduce `InjectionToken<T>` which is a parameterized and type-safe
version of `OpaqueToken`.
DEPRECATION:
- `OpaqueToken` is now deprecated, use `InjectionToken<T>` instead.
- `Injector.get(token: any, notFoundValue?: any): any` is now deprecated
use the same method which is now overloaded as
`Injector.get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T): T;`.
Migration
- Replace `OpaqueToken` with `InjectionToken<?>` and parameterize it.
- Migrate your code to only use `Type<?>` or `InjectionToken<?>` as
injection tokens. Using other tokens will not be supported in the
future.
BREAKING CHANGE:
- Because `injector.get()` is now parameterize it is possible that code
which used to work no longer type checks. Example would be if one
injects `Foo` but configures it as `{provide: Foo, useClass: MockFoo}`.
The injection instance will be that of `MockFoo` but the type will be
`Foo` instead of `any` as in the past. This means that it was possible
to call a method on `MockFoo` in the past which now will fail type
check. See this example:
```
class Foo {}
class MockFoo extends Foo {
setupMock();
}
var PROVIDERS = [
{provide: Foo, useClass: MockFoo}
];
...
function myTest(injector: Injector) {
var foo = injector.get(Foo);
// This line used to work since `foo` used to be `any` before this
// change, it will now be `Foo`, and `Foo` does not have `setUpMock()`.
// The fix is to downcast: `injector.get(Foo) as MockFoo`.
foo.setUpMock();
}
```
- Introduce `InjectionToken<T>` which is a parameterized and type-safe
version of `OpaqueToken`.
DEPRECATION:
- `OpaqueToken` is now deprecated, use `InjectionToken<T>` instead.
- `Injector.get(token: any, notFoundValue?: any): any` is now deprecated
use the same method which is now overloaded as
`Injector.get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T): T;`.
Migration
- Replace `OpaqueToken` with `InjectionToken<?>` and parameterize it.
- Migrate your code to only use `Type<?>` or `InjectionToken<?>` as
injection tokens. Using other tokens will not be supported in the
future.
BREAKING CHANGE:
- Because `injector.get()` is now parameterize it is possible that code
which used to work no longer type checks. Example would be if one
injects `Foo` but configures it as `{provide: Foo, useClass: MockFoo}`.
The injection instance will be that of `MockFoo` but the type will be
`Foo` instead of `any` as in the past. This means that it was possible
to call a method on `MockFoo` in the past which now will fail type
check. See this example:
```
class Foo {}
class MockFoo extends Foo {
setupMock();
}
var PROVIDERS = [
{provide: Foo, useClass: MockFoo}
];
...
function myTest(injector: Injector) {
var foo = injector.get(Foo);
// This line used to work since `foo` used to be `any` before this
// change, it will now be `Foo`, and `Foo` does not have `setUpMock()`.
// The fix is to downcast: `injector.get(Foo) as MockFoo`.
foo.setUpMock();
}
```
PR Close angular#13785
- Introduce `InjectionToken<T>` which is a parameterized and type-safe
version of `OpaqueToken`.
DEPRECATION:
- `OpaqueToken` is now deprecated, use `InjectionToken<T>` instead.
- `Injector.get(token: any, notFoundValue?: any): any` is now deprecated
use the same method which is now overloaded as
`Injector.get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T): T;`.
Migration
- Replace `OpaqueToken` with `InjectionToken<?>` and parameterize it.
- Migrate your code to only use `Type<?>` or `InjectionToken<?>` as
injection tokens. Using other tokens will not be supported in the
future.
BREAKING CHANGE:
- Because `injector.get()` is now parameterize it is possible that code
which used to work no longer type checks. Example would be if one
injects `Foo` but configures it as `{provide: Foo, useClass: MockFoo}`.
The injection instance will be that of `MockFoo` but the type will be
`Foo` instead of `any` as in the past. This means that it was possible
to call a method on `MockFoo` in the past which now will fail type
check. See this example:
```
class Foo {}
class MockFoo extends Foo {
setupMock();
}
var PROVIDERS = [
{provide: Foo, useClass: MockFoo}
];
...
function myTest(injector: Injector) {
var foo = injector.get(Foo);
// This line used to work since `foo` used to be `any` before this
// change, it will now be `Foo`, and `Foo` does not have `setUpMock()`.
// The fix is to downcast: `injector.get(Foo) as MockFoo`.
foo.setUpMock();
}
```
PR Close angular#13785
Adds an overload to TestBed.get making parameters strongly typed and deprecated previous signature that accepted types `any`. The function still returns `any` to prevent build breakages, but eventually stronger type checks will be added so a future Angular version will break builds due to additional type checks. See previous breaking change - #13785 Issue #26491 PR Close #29290
Adds an overload to TestBed.get making parameters strongly typed and deprecated previous signature that accepted types `any`. The function still returns `any` to prevent build breakages, but eventually stronger type checks will be added so a future Angular version will break builds due to additional type checks. See previous breaking change - angular#13785 Issue angular#26491 PR Close angular#29290
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
- Introduce `InjectionToken<T>` which is a parameterized and type-safe
version of `OpaqueToken`.
DEPRECATION:
- `OpaqueToken` is now deprecated, use `InjectionToken<T>` instead.
- `Injector.get(token: any, notFoundValue?: any): any` is now deprecated
use the same method which is now overloaded as
`Injector.get<T>(token: Type<T>|InjectionToken<T>, notFoundValue?: T): T;`.
Migration
- Replace `OpaqueToken` with `InjectionToken<?>` and parameterize it.
- Migrate your code to only use `Type<?>` or `InjectionToken<?>` as
injection tokens. Using other tokens will not be supported in the
future.
BREAKING CHANGE:
- Because `injector.get()` is now parameterize it is possible that code
which used to work no longer type checks. Example would be if one
injects `Foo` but configures it as `{provide: Foo, useClass: MockFoo}`.
The injection instance will be that of `MockFoo` but the type will be
`Foo` instead of `any` as in the past. This means that it was possible
to call a method on `MockFoo` in the past which now will fail type
check. See this example:
```
class Foo {}
class MockFoo extends Foo {
setupMock();
}
var PROVIDERS = [
{provide: Foo, useClass: MockFoo}
];
...
function myTest(injector: Injector) {
var foo = injector.get(Foo);
// This line used to work since `foo` used to be `any` before this
// change, it will now be `Foo`, and `Foo` does not have `setUpMock()`.
// The fix is to downcast: `injector.get(Foo) as MockFoo`.
foo.setUpMock();
}
```
PR Close angular#13785
fix(di): add better type information to injector.get()
BREAKING CHANGE:
injector.get()is now parameterize and returns a type based on args.injector.get(Foo)returns an instance ofFooinjector.get(new OpaqueToken<Foo>(‘Foo’))returns an instance ofFoo.Migration
OpaqueToken, it must now be parameterized.type other than itself. For example
var cars: Car[] = injector.get(Car)because the provider forCarwas declared
multi: true. This is now no longer possible and it isconsidered a bad form. Proper way to do this is through
var CARS = new OpaqueToken<Car[]>;and thenvar cars = injector.get(CARS).var foo: any = injector.get(‘something’)is still supported andresults in
anytype, but it’s use is strongly discouraged.