-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Instantiation of intersections not checked for abstract restriction #4559
Comments
Let's say my IMO, this makes sense since abstract simply doesn't work with the type arithmetic. Once you start claiming to be merging types, the abstract restriction on the ctor is gone since we don't know the semantics of how the merge has been performed. |
@weswigham - here's the mental model I've been using: abstract methods on an abstract class are kinda like imports of a module. Once you satisfy the imports, you know you can safely use the module. In the case of abstract classes, once you've implemented the abstract parts, you know it's safe to execute the class. If we merge to partial classes together that satisfy each other's requirements, it should be possible to create something that can be safely constructed and used. |
I don't see why it couldn't. Here's the idea: Union typesIf given A | B, if either A or B are abstract, then A | B is abstract as well. This means that any value of type A | B is not constructable. Intersection typesIf given A & B, any property one of A or B that is abstract produces an analogous property in A & B unless there exists in a respective property in the other type that is not abstract. If any one property in A & B is abstract, then A & B itself becomes abstract, and any value of type A & B is not constructable. |
Yes, we can say that semantically that's how it can work, but think about how actual intersection types get made in JS - it's perfectly reasonable to take in two "abstract" classes and create a concrete one from them. How do I represent that in the typesystem if abstractness starts propagating with the types? |
Like if I actually implement that function merge<A, B>(a: A, b: B) : A & B {
let merged = function() {
a.call(this);
b.call(this);
}
Object.assign(merged.prototype, a.prototype, b.prototype);
return merged;
} The result is never going to be "abstract", in the strict sense, and given the implementation, it's perfectly reasonable to expect to be able to construct the result (despite any grievances about missing abstract members). So if I want to say 'Yes, the result is constructable, despite remaining potential abstractness', how do I indicate that in this new scenario? |
I think you meant to return a type that produces the intersection when constructed, not the intersection of two constructors.
Why is that perfectly reasonable? A major utility of
You merge it with an object that has a concrete implementation, or you add a type annotation/assertion for a new type that doesn't have abstract members. You should need an escape hatch to say "yes, I know what I'm doing" here. |
Yeah; I was just copying the declaration line from above, sorry. Rewriting it, I found it impossible to type it correctly without using an function merge<A extends new() => void, B extends new() => void>(a: A, b: B) : new() => A & B {
let merged = function() {
a.call(this);
b.call(this);
}
Object.assign(merged.prototype, a.prototype, b.prototype);
return <new() => A & B><any>merged;
} Which feels awkward.
Yeah - what I'm trying to say is that we don't have that escape hatch for types built from intersection types. Unless we're considering casts to |
Pretty much comes down to resolving conflicts #4805 |
We will need a concrete example for what scenarios this would be blocking, ideally with a real implementation of the mixin function, rather than a hypothetical one. |
Say I wanted to create two partial classes, merge them together, and instantiate the result:
We currently don't catch the error that we're new'ing something that is abstract. Notice the 'rright' in the Right, so that intersection can't really satisfy the abstract requirements from Left.
The text was updated successfully, but these errors were encountered: