Skip to content
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

TSX spread stopped working #14112

Closed
normalser opened this issue Feb 16, 2017 · 10 comments
Closed

TSX spread stopped working #14112

normalser opened this issue Feb 16, 2017 · 10 comments
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue

Comments

@normalser
Copy link

import * as React from "react";

interface Props {

}

export class Test extends React.Component<Props, {}> {
    render() {
        return <div {...this.props} />
    }
}

Was working fine until: [email protected]

In [email protected] throws:

error TS2698: Spread types may only be created from object types.

Not sure if related: #13557

@yuit
Copy link
Contributor

yuit commented Feb 16, 2017

@normalser it is related to #13557 as now spread operation in Jsx is going through same logic as normal spread (previously it is handled separately... I will discuss with @sandersn who is working in this area

@yuit yuit added the Domain: JSX/TSX Relates to the JSX parser and emitter label Feb 16, 2017
@sandersn
Copy link
Member

It will be fixed by #13288. Reviews are welcome!

The original proposal is #10727, which is the main bug for this issue.

@sandersn sandersn added the Duplicate An existing issue was already created label Feb 16, 2017
@yuit yuit removed the Duplicate An existing issue was already created label Feb 16, 2017
@yuit yuit reopened this Feb 16, 2017
@sandersn
Copy link
Member

In #13288, getSpreadType has a special case that flattens intersections if they only contain object types (no type parameters, etc). I don't know React, but spreading this.props looks like it is extremely common. If so, it would be worthwhile to ship that part of the change early.

@normalser @yuit @RyanCavanaugh opinions from people who actually know React?

@sandersn
Copy link
Member

Note that this would make error reporting a lot harder (impossible?) for the case that the intersection does contain a type parameter.

@sandersn
Copy link
Member

After discussing with @yuit, I think the fix is just to get #13288 checked in soon so that spread types are fully supported. Supporting intersections separately would be too error-prone.

@sandersn
Copy link
Member

Well, it turns out that the culprit is that checkObjectLiteral and createJsxAttributesType both call getSpreadType and both check for object types. But createJsxAttributesType doesn't call isValidSpreadType, it uses an older, simpler check. So this is an easy fix, after all: just make anybody who calls getSpreadType first call isValidSpreadType.

@RyanCavanaugh
Copy link
Member

JSX spread is very common; this needs to work for a major release

@sandersn
Copy link
Member

Fix is up at #14122

@mhegazy mhegazy added the Bug A bug in TypeScript label Feb 16, 2017
@mhegazy mhegazy added this to the TypeScript 2.3 milestone Feb 16, 2017
@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Feb 16, 2017
@stephen
Copy link

stephen commented Apr 27, 2017

JSX spread is very common; this needs to work for a major release

Is there a workaround for folks wanting to use ts 2.3 with spread types? It seems like the common react pattern of spreading this.props will no longer work. Example code:

class Component<T extends object> extends React.Component<T, {}> {
  render() { return <div { ...this.props }></div>; }
}

compiler output:

ERROR in ./file.tsx
(34,17): error TS2698: Spread types may only be created from object types.

@yuit
Copy link
Contributor

yuit commented May 5, 2017

@stephen we are going to take a look about this and see what we can do. We have this PR (https://github.com/Microsoft/TypeScript/pull/13288/files) for normal spread which we will want to get in and apply for React

Update we have lifted up the restriction.... The fix is in tonight nightly. we will also ship this for 2.3.3

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

6 participants