Regression: 2.4.1 Generic type mismatch #16985
Comments
|
I'm having a similar problem, I think. interface ModelData {
type: string;
attributes: { [key: string]: any };
}
class Model<MD extends ModelData> {
// stuff
static type = 'abstract';
set(v: MD): this {
// set value to v
return this; // for chaining
}
}
class Registry {
private reg: { [key: string]: typeof Model };
add(M: typeof Model) {
reg[M.type] = M;
}
}
interface TestData extends ModelData {
type: 'test';
attributes: {
foo: string;
};
}
class TestModel extends Model<TestData> {
static type = 'test';
}
const reg = new Registry();
reg.add(TestModel); // ERROR in TSC 2.4.1, not 2.4.0At least, I think this is the same sort of problem - I want to express that my registry contains constructors for Model objects, and that Model.set() returns this for chaining. Changing |
|
@ericeslinger - yeah, definitely looks like the same kind of problem. Using |
|
This fixes the urgent symptom of the problem, but doesn't seem to address the underlying problem. To wit: the new generic checks don't seem to play nicely with class generics and subclasses. My entire data model is predicated on having a generic Model class to represent a data model and its actions (get / set / reset / etc), and then the user subclassing Model with a concrete instance of Schema, so we know the specific shape of the data returned by get(). I'd prefer to write canonical TS, not stuff that only builds if you turn off default compiler settings. |
|
I think the issue here is that (diff) // ...
+declare class AnyModel extends Model<ModelData> {};
class Registry {
- private reg: { [key: string]: typeof Model };
+ private reg: { [key: string]: typeof AnyModel };
- add(M: typeof Model) {
+ add(M: typeof AnyModel) {
- reg[M.type] = M;
+ this.reg[M.type] = M;
}
}
// ...
const reg = new Registry();
-reg.add(TestModel); // ERROR in TSC 2.4.1, not 2.4.0
+reg.add(TestModel); // no error(entire) interface ModelData {
type: string;
attributes: { [key: string]: any };
}
class Model<MD extends ModelData> {
// stuff
static type = 'abstract';
set(v: MD): this {
// set value to v
return this; // for chaining
}
}
declare class AnyModel extends Model<ModelData> {};
class Registry {
private reg: { [key: string]: typeof AnyModel };
add(M: typeof AnyModel) {
this.reg[M.type] = M;
}
}
interface TestData extends ModelData {
type: 'test';
attributes: {
foo: string;
};
}
class TestModel extends Model<TestData> {
static type = 'test';
}
const reg = new Registry();
reg.add(TestModel); // no error |
|
I'm having a similar issue. Seems like stricter checks on generics and interface IDefinition { }
class DefinitionBase implements IDefinition { }
class Def1 extends DefinitionBase { }
class Def2 extends DefinitionBase { }
// ---------------------- usage ----------------------
interface ITest<D extends IDefinition> {
definition: D;
}
abstract class TestBase<DImpl extends DefinitionBase> implements ITest<DImpl> {
abstract definition: DImpl;
}
class ConcreteTest1 extends TestBase<Def1> {
definition: Def1;
}
class ConcreteTest2 extends TestBase<Def2> {
definition: Def2;
}
// classes container
class AllTestClasses<T extends typeof TestBase> {
array: T[];
}
// ---------------------- fail ----------------------
var t = new AllTestClasses();
t.array.push(ConcreteTest1); // Def1 vs. DImpl error
t.array.push(ConcreteTest2); // Def2 vs. DImpl errorErrors: The only way to make it work with TS 2.4.1 is to use |
|
@mat3e That error actually looks right to me, but my attempted "fix" for it produces an even stranger error, IMHO. Changing // classes container
class AllTestClasses {
array: TestBase<DefinitionBase>[];
}results in:
|
// ...
+declare class AnyTestBase extends TestBase<DefinitionBase> {
+ definition: DefinitionBase;
+};
// ...
-class AllTestClasses<T extends typeof TestBase> {
+class AllTestClasses<T extends typeof AnyTestBase> {
array: T[];
}
// ...
|
|
Reduced reproduction: interface MyInterface {
something: number;
}
var doIt = <T extends MyInterface>(d: Partial<T>) => {}
doIt = (d: Partial<MyInterface>) => {} |
|
Here's another reproduction: class Base<P = {}> {
constructor(protected p: P) { }
}
class Baz extends Base { }
const factory = (Clazz: typeof Base) => {
return class extends Clazz { }
}
const xs = factory(Baz)Error message: |
|
(See similar #16985 (comment) ) If I have no idea when-or-if this is-or-should-be a valid inference. |
|
@estaub , I think this is a bug about type inference, since I can compile these code successfully in Scala: class Base { }
class Foo extends Base { }
def checkType(c: Class[_ <: Base]) = println("Get Class Type: " + c.getName)
val clazz1 = (new Foo).getClass
checkType(clazz1) // Get Class Type: Playground$Fooclass Base[+A] { }
class Foo[A] extends Base[A] { }
class Baz { }
class Bar extends Baz { }
def checkType[A <: Baz](c: Class[_ <: Base[A]]) = println("Get Class Typ1e: " + c.getName)
val clazz1 = (new Foo[Bar]).getClass
checkType[Baz](clazz1) |
This is true, but if there are some generics, for example: If If This is all about how strict it should be, I have no idea if this is a bug or not.
class Base<P = {}> {
constructor(public p: P) { }
}
class AnotherBase<P> {
constructor(public p: P) { }
}
class Baz extends Base {}
const factory = (Clazz: typeof Base) => {
return class extends Clazz {}
}
const xs = factory(AnotherBase) // passedclass Base<P = {}> {
constructor(protected p: P) { }
}
class Foo extends Base {}
class Baz extends Base {}
const factory = (Clazz: typeof Foo) => {
return class extends Clazz {}
}
const xs = factory(Baz) // passed |
|
@ikatyang I fully understand, but I still think the value of |
|
Ah.., It seems the previous behavior inferred all generics as I'm not sure if this is the expected behavior, since |
|
@ikatyang , Mmm, thank you for your explanation, I think I have some misunderstanding about class Base<P = {}> {
constructor(protected p: P) { }
}
class Baz<P = {}> extends Base<P> { }
const factory = (Clazz: typeof Base) => {
return class extends Clazz { }
}
const xs = factory(Baz) |
|
Potentially similar issue: // rxjs mapTo
export function mapTo<T, R>(this: Observable<T>, value: R): Observable<R> {
return this.lift(new MapToOperator(value));
}
// Boolean issues
const trueObs$: Observable<true> = someObs$.mapTo(true);
const falseObs$: Observable<false> = otherObs$.mapTo(false);yields Removing the types works ( |
|
That seems another issue about literal-type widening, for example: const x1 = {v: true}; //=> {v: boolean}
const x2 = {v: true as true}; //=> {v: true}interface X<T> {
v: T;
}
declare function getX<T>(v: T): X<T>;
getX(true); //=> X<boolean>
getX<true>(true); //=> X<true> |
|
Indeed, it just seems intuitive to me that by supplying an explicit type ( This syntactic detail becomes more noticeable to me with a function accepting more than one generic parameter. In that case it becomes less clear what Side by side in code it looks a bit silly, no?: interface X<T> {
v: T
}
const getX = <T>(v: T): X<T> => ({v});
const x: X<true> = getX(true);
// Fail
const getY = <T>(v: T): T => v;
const y: true = getY(true);
// Success! |
|
Seems like that this same problem export type Foo = {
x: number;
}
export class A<T extends Foo> {
constructor() {
let m: T = {x: 1};
}
}output Updated |
|
@izatop it seems working as intended, consider the following case: const a = new A<{y: number}>();
// `T` is `{y: number}` in this case, `{x: 1}` is not assignable to `T`and default generic does nothing here, it is just a default type. Only |
|
@ikatyang sorry, I wrote wrong example. See updated comment. |
Super-type of
const a = new A<{x: number, y: number}>();
// `T` is `{x: number, y: number}` in this case, `{x: 1}` is not assignable to `T` |
|
@ikatyang thanks, now it seems to clear. |
|
@ikatyang, what I don't get in your fix to my problem is why creating a class if needed, I'd prefer to type the registry as For now, I guess I'll just disable strict generic checking or do some kludgy workaround, but I'd be interested in hearing if this is a compiler bug or if it is just me not thinking correctly about how one
|
|
As I mentioned above, this is caused by stricter generic checks (#16368). Before that PR, they squashed all generics to After that PR, all generics are still own its original shape, thus I'm not sure if this is Working as intended or a bug, since |
|
Yeah, I think the specific issue here is how the type of a polymorphic this return is computed. EDIT: Nevermind - just read my error message more clearly. It's the argument to |
DanielRosenwasser
added
the
Needs Investigation
DanielRosenwasser
added
Bug
|
I'll note that |
|
(Note: Issue linked above is for an infinite recursion crash in the compiler that most likely occurs due to the same changes in the compiler, since solving the original issue here also solved the crash. Looks like this code in the compiler isn't 100% correct, given the number of infinite recursion bugs lately). My understanding is that most people here feel that this should have been a breaking change, because given But in v2.4.1, if Whether This is aggravated by the fact that this issue extends to existing .d.ts files which no longer work. Less contrived example: /** A generic UI component that renders to a DOM element */
abstract class UIComponent<RenderedElementT extends HTMLElement> {
public static staticMethod() { /* ... */ }
// more static methods here ...
public abstract render(): RenderedElementT;
// ...
}
class TextField extends UIComponent<HTMLInputElement> {
public render() { return document.createElement("input") }
// ...
}
function findComponentsByType(ComponentClass: typeof UIComponent) {
// The issue here is that e.g. TextField is not assignable to `typeof UIComponent`
// but then how do we make sure that only a "component class" can be passed in?
// other than having to resort to a "class object" interface type that includes
// a constructor and all static methods, which needs to be declared separately
}
var results = findComponentsByType(TextField); |
|
The issue in the OP is that the mapped type relationship implemented in #13448 doesn't correctly handle the case where interface Foo { a: string, b: string }
function f<T extends Foo>(a: Partial<Foo>, b: Partial<T>) {
a = b; // Error, but shouldn't be
}With the stricter generic checks introduced in #16368 we no longer erase type parameters to type |
|
Hi, since TS 2.4.1, I'm facing errors when using type inference with generic functions. Example: type transformerFn = <T, U>(input: T) => U;
const toUpper: transformerFn = (str: string): string => str.toUpperCase();Expected result: Actual result:
Affected versions: Workaround: I use this pattern quite often to describe a common type interface, which can be configured with concrete implementations. And therefore, I relying on the type inference system in these cases. Can someone please tell me if this is related to this issue or if I should open a new issue for the problem, of if this is an error at all!? Thanks in advance 😄 |
|
@awaltert I'm no expert, but no, that doesn't appear to be the same problem. As in so many other places, once you tell Typescript the type of something, it won't do any inferencing to refine that definition. I wish there was a way to say "go ahead and refine my definition by inferencing", but there ain't. The solution is to not provide the type (where inferencing is possible, as here), or provide it in all the details required downstream. Also, the
|
|
@estaub Thanks for the quick response. 👍 With the given code example, I tried to reduce my problem to a basic example, which could lead to misunderstandings, sorry for that. The explicit type declaration In general, you can provide the "variable declaration" for the generic type arguments on the right side of the equal sign. The difference compared with the left side is:
It is used to declare the type variable, which are then known in the scope of the function. Please have a look at the example below. Because of this mechanism, the concrete types can be inferred 😄 const toLength = (str: string) => str.length; // string => number
type mappable = <T, U>(transformer: (input: T) => U) => (inputArray: T[]) => U[]; // use type inference
const mappable: mappable = (transformer) => (input) => input.map(transformer);
const lengths = mappable(toLength); // inferred as lengths:: string[] => number[]
console.log(lengths(["generics", "type", "inference"])); // outputs: [ 8, 4, 9 ]So my question still remains: Why is the error As this issue is already fixed, should I create a new issue for discussion purpose? |
|
@awaltert The error is correct. The type type transformerFn = <T, U>(input: T) => Umeans the function Your second example is actually substantially different. type mappable = <T, U>(transformer: (input: T) => U) => (inputArray: T[]) => U[];reads: The biggest difference is that when you're annotating a variable as in your first example, your opting in for the generic version, i.e. "for every Edit: To exemplify the difference, consider that instead of the implementation you've provided for const mappable: mappable = (transformer: (x: string) => string) => (input: string[]) => input.map(transformer);Would that have had been correct? |
|
Wait hang on, that's a super specific solution (as mentioned by @DanielRosenwasser on the PR...) How would this solve the issue with /** A generic UI component that renders to a DOM element */
abstract class UIComponent<RenderedElementT extends HTMLElement> {
public static staticMethod() { /* ... */ }
// more static methods here ...
public abstract render(): RenderedElementT;
// ...
}
class TextField extends UIComponent<HTMLInputElement> {
public render() { return document.createElement("input") }
// ...
}
function findComponentsByType(ComponentClass: typeof UIComponent) {
// THIS::::
// The issue here is that e.g. TextField is not assignable to `typeof UIComponent`
// but then how do we make sure that only a "component class" can be passed in?
// other than having to resort to a "class object" interface type that includes
// a constructor and all static methods, which needs to be declared separately
}
var results = findComponentsByType(TextField);Or do we open up another issue for this / is there another issue open to track this? |
|
@jcormont The error reported in the example above is intended and is one the checker wasn't capable of finding before #16368. In the example, new<RenderedElementT extends HTMLElement>(): UIComponent<RenderedElementT>;but this contract isn't satisfied by the constructor function signature in new(): TextField;In intuitive terms, the generic constructor in There are several ways you could structure a correct implementation. One would be to write an actual interface declaration for the parameter of interface ComponentFactory {
new(): UIComponent<HTMLElement>;
}
function findComponentsByType(ComponentClass: ComponentFactory) {
// ...
}Alternatively you could introduce a base class with a constructor contract that is satisfied by all derived classes, e.g. abstract class ComponentBase {
public abstract render(): HTMLElement;
// ...
}
abstract class UIComponent<RenderedElementT extends HTMLElement> extends ComponentBase {
public abstract render(): RenderedElementT;
// ...
}
class TextField extends UIComponent<HTMLInputElement> {
public render() { return document.createElement("input") }
// ...
}
function findComponentsByType(ComponentClass: typeof ComponentBase) {
// ...
}Either way, within the implementation of |
|
Thanks, that makes sense now. It's still not very intuitive, especially with abstract classes (where the constructor isn't callable anyway so you would usually pass the Since the interface ComponentFactory extends typeof UIComponent { // <==== why not
// all static methods/properties inherited from UIComponent
// plus constructor without type parameters:
new(): UIComponent<HTMLElement>;
}
function findComponentsByType(ComponentClass: ComponentFactory) {
ComponentClass.doSomething(); // <== static method?
// ...
} |
|
An type PublicMembersOf<T> = { [P in keyof T]: T[P] };
interface ComponentFactory extends PublicMembersOf<typeof UIComponent> {
new(): UIComponent<HTMLElement>;
} |
This is a simplified piece of code which demonstrates the problem. It compiles fine in 2.3.4 & 2.4.0, but produces a not assignable error in 2.4.1 and nightly.
TypeScript Version: 2.4.1 / nightly (2.5.0-dev.20170629)
Code
Expected behavior:
No compile error.
Actual behavior:
The text was updated successfully, but these errors were encountered: