Skip to content

Fixed JSX attributes discriminating based on optional children #53980

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 26, 2023
Merged

Fixed JSX attributes discriminating based on optional children #53980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b5e2877
Fixed JSX attributes discriminating based on optional children
Andarist Apr 24, 2023
862f863
fixed as issue with self-closing elements
Andarist Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29336,14 +29336,24 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function discriminateContextualTypeByJSXAttributes(node: JsxAttributes, contextualType: UnionType) {
const jsxChildrenPropertyName = getJsxElementChildrenPropertyName(getJsxNamespaceAt(node));
return discriminateTypeByDiscriminableItems(contextualType,
concatenate(
map(
filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.JsxAttribute && isDiscriminantProperty(contextualType, p.symbol.escapedName) && (!p.initializer || isPossiblyDiscriminantValue(p.initializer))),
Copy link
Contributor Author

@Andarist Andarist Apr 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came to the conclusion that this "branch" doesn't need any immediate adjustments because JSX expressions can't be easily used as value-based discriminants.

  1. JsxText is always typed as stringType (no string literal types there)
  2. more than 1 children also can't be a discriminant value because arrays~ can't be such

In theory, it's actually possible to have things like:

type Props = {
  children: "a"
  foo: string
} | {
  children: "b"
  foo: number
}

;<Comp foo="bar">{"a"}</Comp>

So maybe this should be improved further, for this very specific use case - just to be consistent with:

;<Comp foo="bar" children={"a"} />

I think though that this can be done in a separate PR (if requested). It would be somewhat weird though that those 2 wouldn't behave the same, so perhaps further adjustments (beyond this function here) should be introduced in such a PR:

;<Comp foo="bar">{"a"}</Comp>
;<Comp foo="bar">a</Comp>

Note that this doesn't work in 5.0 anyway so likely I didn't regress this in #53502: TS playground 5.0

prop => ([!(prop as JsxAttribute).initializer ? (() => trueType) : (() => getContextFreeTypeOfExpression((prop as JsxAttribute).initializer!)), prop.symbol.escapedName] as [() => Type, __String])
),
map(
filter(getPropertiesOfType(contextualType), s => !!(s.flags & SymbolFlags.Optional) && !!node?.symbol?.members && !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName)),
filter(getPropertiesOfType(contextualType), s => {
if (!(s.flags & SymbolFlags.Optional) || !node?.symbol?.members) {
return false;
}
const element = node.parent.parent;
if (s.escapedName === jsxChildrenPropertyName && isJsxElement(element) && getSemanticJsxChildren(element.children).length) {
return false;
}
return !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName);
}),
s => [() => undefinedType, s.escapedName] as [() => Type, __String]
)
),
Expand Down
114 changes: 114 additions & 0 deletions tests/baselines/reference/contextuallyTypedJsxChildren.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
=== tests/cases/compiler/contextuallyTypedJsxChildren.tsx ===
/// <reference path="react16.d.ts" />

import React from 'react';
>React : Symbol(React, Decl(contextuallyTypedJsxChildren.tsx, 2, 6))

// repro from https://github.com/microsoft/TypeScript/issues/53941
declare namespace DropdownMenu {
>DropdownMenu : Symbol(DropdownMenu, Decl(contextuallyTypedJsxChildren.tsx, 2, 26), Decl(contextuallyTypedJsxChildren.tsx, 23, 13))

interface BaseProps {
>BaseProps : Symbol(BaseProps, Decl(contextuallyTypedJsxChildren.tsx, 5, 32))

icon: string;
>icon : Symbol(BaseProps.icon, Decl(contextuallyTypedJsxChildren.tsx, 6, 23))

label: string;
>label : Symbol(BaseProps.label, Decl(contextuallyTypedJsxChildren.tsx, 7, 17))
}
interface PropsWithChildren extends BaseProps {
>PropsWithChildren : Symbol(PropsWithChildren, Decl(contextuallyTypedJsxChildren.tsx, 9, 3))
>BaseProps : Symbol(BaseProps, Decl(contextuallyTypedJsxChildren.tsx, 5, 32))

children(props: { onClose: () => void }): JSX.Element;
>children : Symbol(PropsWithChildren.children, Decl(contextuallyTypedJsxChildren.tsx, 10, 49))
>props : Symbol(props, Decl(contextuallyTypedJsxChildren.tsx, 11, 13))
>onClose : Symbol(onClose, Decl(contextuallyTypedJsxChildren.tsx, 11, 21))
>JSX : Symbol(JSX, Decl(react16.d.ts, 2493, 12))
>Element : Symbol(JSX.Element, Decl(react16.d.ts, 2494, 23))

controls?: never | undefined;
>controls : Symbol(PropsWithChildren.controls, Decl(contextuallyTypedJsxChildren.tsx, 11, 58))
}
interface PropsWithControls extends BaseProps {
>PropsWithControls : Symbol(PropsWithControls, Decl(contextuallyTypedJsxChildren.tsx, 13, 3))
>BaseProps : Symbol(BaseProps, Decl(contextuallyTypedJsxChildren.tsx, 5, 32))

controls: Control[];
>controls : Symbol(PropsWithControls.controls, Decl(contextuallyTypedJsxChildren.tsx, 14, 49))
>Control : Symbol(Control, Decl(contextuallyTypedJsxChildren.tsx, 17, 3))

children?: never | undefined;
>children : Symbol(PropsWithControls.children, Decl(contextuallyTypedJsxChildren.tsx, 15, 24))
}
interface Control {
>Control : Symbol(Control, Decl(contextuallyTypedJsxChildren.tsx, 17, 3))

title: string;
>title : Symbol(Control.title, Decl(contextuallyTypedJsxChildren.tsx, 18, 21))
}
type Props = PropsWithChildren | PropsWithControls;
>Props : Symbol(Props, Decl(contextuallyTypedJsxChildren.tsx, 20, 3))
>PropsWithChildren : Symbol(PropsWithChildren, Decl(contextuallyTypedJsxChildren.tsx, 9, 3))
>PropsWithControls : Symbol(PropsWithControls, Decl(contextuallyTypedJsxChildren.tsx, 13, 3))
}
declare const DropdownMenu: React.ComponentType<DropdownMenu.Props>;
>DropdownMenu : Symbol(DropdownMenu, Decl(contextuallyTypedJsxChildren.tsx, 2, 26), Decl(contextuallyTypedJsxChildren.tsx, 23, 13))
>React : Symbol(React, Decl(contextuallyTypedJsxChildren.tsx, 2, 6))
>ComponentType : Symbol(React.ComponentType, Decl(react16.d.ts, 117, 60))
>DropdownMenu : Symbol(DropdownMenu, Decl(contextuallyTypedJsxChildren.tsx, 2, 26), Decl(contextuallyTypedJsxChildren.tsx, 23, 13))
>Props : Symbol(DropdownMenu.Props, Decl(contextuallyTypedJsxChildren.tsx, 20, 3))

<DropdownMenu icon="move" label="Select a direction">
>DropdownMenu : Symbol(DropdownMenu, Decl(contextuallyTypedJsxChildren.tsx, 2, 26), Decl(contextuallyTypedJsxChildren.tsx, 23, 13))
>icon : Symbol(icon, Decl(contextuallyTypedJsxChildren.tsx, 25, 13))
>label : Symbol(label, Decl(contextuallyTypedJsxChildren.tsx, 25, 25))

{({ onClose }) => (
>onClose : Symbol(onClose, Decl(contextuallyTypedJsxChildren.tsx, 26, 5))

<div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2546, 114))

<button onClick={onClose}>Click me</button>
>button : Symbol(JSX.IntrinsicElements.button, Decl(react16.d.ts, 2532, 96))
>onClick : Symbol(onClick, Decl(contextuallyTypedJsxChildren.tsx, 28, 13))
>onClose : Symbol(onClose, Decl(contextuallyTypedJsxChildren.tsx, 26, 5))
>button : Symbol(JSX.IntrinsicElements.button, Decl(react16.d.ts, 2532, 96))

</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2546, 114))

)}
</DropdownMenu>;
>DropdownMenu : Symbol(DropdownMenu, Decl(contextuallyTypedJsxChildren.tsx, 2, 26), Decl(contextuallyTypedJsxChildren.tsx, 23, 13))

<DropdownMenu
>DropdownMenu : Symbol(DropdownMenu, Decl(contextuallyTypedJsxChildren.tsx, 2, 26), Decl(contextuallyTypedJsxChildren.tsx, 23, 13))

icon="move"
>icon : Symbol(icon, Decl(contextuallyTypedJsxChildren.tsx, 33, 13))

label="Select a direction"
>label : Symbol(label, Decl(contextuallyTypedJsxChildren.tsx, 34, 13))

children={({ onClose }) => (
>children : Symbol(children, Decl(contextuallyTypedJsxChildren.tsx, 35, 28))
>onClose : Symbol(onClose, Decl(contextuallyTypedJsxChildren.tsx, 36, 14))

<div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2546, 114))

<button onClick={onClose}>Click me</button>
>button : Symbol(JSX.IntrinsicElements.button, Decl(react16.d.ts, 2532, 96))
>onClick : Symbol(onClick, Decl(contextuallyTypedJsxChildren.tsx, 38, 13))
>onClose : Symbol(onClose, Decl(contextuallyTypedJsxChildren.tsx, 36, 14))
>button : Symbol(JSX.IntrinsicElements.button, Decl(react16.d.ts, 2532, 96))

</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2546, 114))

)}
/>;

106 changes: 106 additions & 0 deletions tests/baselines/reference/contextuallyTypedJsxChildren.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
=== tests/cases/compiler/contextuallyTypedJsxChildren.tsx ===
/// <reference path="react16.d.ts" />

import React from 'react';
>React : typeof React

// repro from https://github.com/microsoft/TypeScript/issues/53941
declare namespace DropdownMenu {
interface BaseProps {
icon: string;
>icon : string

label: string;
>label : string
}
interface PropsWithChildren extends BaseProps {
children(props: { onClose: () => void }): JSX.Element;
>children : (props: { onClose: () => void;}) => JSX.Element
>props : { onClose: () => void; }
>onClose : () => void
>JSX : any

controls?: never | undefined;
>controls : undefined
}
interface PropsWithControls extends BaseProps {
controls: Control[];
>controls : Control[]

children?: never | undefined;
>children : undefined
}
interface Control {
title: string;
>title : string
}
type Props = PropsWithChildren | PropsWithControls;
>Props : PropsWithChildren | PropsWithControls
}
declare const DropdownMenu: React.ComponentType<DropdownMenu.Props>;
>DropdownMenu : React.ComponentType<DropdownMenu.Props>
>React : any
>DropdownMenu : any

<DropdownMenu icon="move" label="Select a direction">
><DropdownMenu icon="move" label="Select a direction"> {({ onClose }) => ( <div> <button onClick={onClose}>Click me</button> </div> )}</DropdownMenu> : JSX.Element
>DropdownMenu : React.ComponentType<DropdownMenu.Props>
>icon : string
>label : string

{({ onClose }) => (
>({ onClose }) => ( <div> <button onClick={onClose}>Click me</button> </div> ) : ({ onClose }: { onClose: () => void; }) => JSX.Element
>onClose : () => void
>( <div> <button onClick={onClose}>Click me</button> </div> ) : JSX.Element

<div>
><div> <button onClick={onClose}>Click me</button> </div> : JSX.Element
>div : any

<button onClick={onClose}>Click me</button>
><button onClick={onClose}>Click me</button> : JSX.Element
>button : any
>onClick : () => void
>onClose : () => void
>button : any

</div>
>div : any

)}
</DropdownMenu>;
>DropdownMenu : React.ComponentType<DropdownMenu.Props>

<DropdownMenu
><DropdownMenu icon="move" label="Select a direction" children={({ onClose }) => ( <div> <button onClick={onClose}>Click me</button> </div> )}/> : JSX.Element
>DropdownMenu : React.ComponentType<DropdownMenu.Props>

icon="move"
>icon : string

label="Select a direction"
>label : string

children={({ onClose }) => (
>children : ({ onClose }: { onClose: () => void; }) => JSX.Element
>({ onClose }) => ( <div> <button onClick={onClose}>Click me</button> </div> ) : ({ onClose }: { onClose: () => void; }) => JSX.Element
>onClose : () => void
>( <div> <button onClick={onClose}>Click me</button> </div> ) : JSX.Element

<div>
><div> <button onClick={onClose}>Click me</button> </div> : JSX.Element
>div : any

<button onClick={onClose}>Click me</button>
><button onClick={onClose}>Click me</button> : JSX.Element
>button : any
>onClick : () => void
>onClose : () => void
>button : any

</div>
>div : any

)}
/>;

47 changes: 47 additions & 0 deletions tests/cases/compiler/contextuallyTypedJsxChildren.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// @strict: true
// @jsx: react
// @esModuleInterop: true
// @noEmit: true

/// <reference path="/.lib/react16.d.ts" />

import React from 'react';

// repro from https://github.com/microsoft/TypeScript/issues/53941
declare namespace DropdownMenu {
interface BaseProps {
icon: string;
label: string;
}
interface PropsWithChildren extends BaseProps {
children(props: { onClose: () => void }): JSX.Element;
controls?: never | undefined;
}
interface PropsWithControls extends BaseProps {
controls: Control[];
children?: never | undefined;
}
interface Control {
title: string;
}
type Props = PropsWithChildren | PropsWithControls;
}
declare const DropdownMenu: React.ComponentType<DropdownMenu.Props>;

<DropdownMenu icon="move" label="Select a direction">
{({ onClose }) => (
<div>
<button onClick={onClose}>Click me</button>
</div>
)}
</DropdownMenu>;

<DropdownMenu
icon="move"
label="Select a direction"
children={({ onClose }) => (
<div>
<button onClick={onClose}>Click me</button>
</div>
)}
/>;