Unsoundness with covariant type

charlag commented 3 years ago

Flow version: 0.156.0 (at least since 0.135.0)

Expected behavior

Making type generic does not introduce unsoundness.

Actual behavior

Link to Try-Flow or Github repo: https://flow.org/try/#0PTAEBUCcE9UhTAtgewG4EsB2BzUAiAaj1AAt4EAaUAQwBtbRzJlIBnUAE3VeoAdf41SACgsAF3IAzagGN4oAIIy5vMa3AAeAuAB8oAN7CAkLJViAFGIBcEAJQ3UydBwDcwgL7DhkgK6YZYujImKA+rPCsSmYsGrrmpvCqLDZRiWqaulSodD7wNuC2Bl6e3n4BQSFhEalJkADCtNSsrLE65jK0rDYNTS016a06WTl5dkUeXh29oADKYpBY2P0soOiIvLRI8Jhqispp6hqs84t6hib7qpY2xws49qCOzuNGniVVkZdiLOaY8ADusxOOGWkHMtioeGoeFswg+oN+AKBdyWXx+ENAACZCiBQGISNxQKwSMgfLQOKBpOhaF54Wj6o1muY5ijQZDobC6dEGb1mcDUdyqNjQLj8YTiaTyZTqNSvMIxNABKAAPIAIwAVvAAgAxFig0AAXiKRmytFyN35E2EU2ayMWas1AX1aw2Wx27H6hwdWrEusgoLOxgSV2sKo1Pr9oMKhmMb1p4U+3MRgJZ9vDTvp4Ko+lN5vw0PcnITCL+Kf53ozSYx+kLcOLmdLdpwFbECOrudGmMLIrAYvYErJFKpNK5aRYPSZqeb6db9OztdHtQnrD5KJbbIMHZsULwC-r3OXq7Tjtngs3IxsXZxvYJ-ZJg+l1KAA

Context

Our Mithril definitions use types which looks similar to these:

export interface Vnode<Attrs> {
    attrs: Attrs,
    children: ChildArray,
    dom: HTMLElement,
}

declare interface Mithril {
    <AttrsT>(
        component: Class<MComponent<AttrsT>>,
        attributes: AttrsT,
        children?: Children
    ): Vnode<any>;
}

interface MComponent<+Attrs> {
    /** Creates a view out of virtual elements. */
    view(vnode: Vnode<Attrs>): Children;
}

export type Child = Vnode<any> | string | number | boolean | null;
export type ChildArray = Array<Children>;
export type Children = Child | ChildArray;

declare var m: Mithril;

// --
// --
// --

type TestAttrs = {
    test: number,
}

class TestComponent implements MComponent<TestAttrs> {
    view(vnode: Vnode<TestAttrs>): Children {
        return String(vnode.attrs.test)
    }
}

function test(): Children {
    return m(TestComponent, {
        test: "c" // this should not be allowed
    })
}

dsainati1 commented 3 years ago

Yep, this is a bug. We should not allow you to make Attrs covariant in the definition of MComponent, since it appears in an input position in the interface's definition.

charlag commented 3 years ago

Ah, of course, it should be contravariant, thanks!

charlag commented 3 years ago

This might be another issue but even when I declare it as contravariant (interface MComponent<-Attrs>) it accepts things like

m(TestComponent, {})

but at least not

m(TestComponent, { another: 2 })

jamesisaac commented 3 years ago

Issue from your last message is likely due to Flow treating {} as an unsealed object. You can spread null to achieve an empty object without it being unsealed: { ...null }. See #7424

facebook / flow