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 data-* props issue with Stricter Assignability Checks to Unions with Index Signatures #36935

Closed
jraoult opened this issue Feb 21, 2020 · 4 comments · Fixed by #36952
Closed
Assignees
Labels
Bug A bug in TypeScript

Comments

@jraoult
Copy link

jraoult commented Feb 21, 2020

Seems like the new Stricter Assignability Checks to Unions with Index Signatures in 3.8 makes it impossible to have data-* props passed to React components when their props type is based on a union.

Pardon the contrived example below (I tried to reduce as much as possible for clarity sake). For a real case most material UI components props types are using unions and therefor fail the same test.

TypeScript Version: 3.8-Beta

Search Terms:
data
Stricter Assignability Checks to Unions with Index Signatures

Expected behavior:
The data- props should be accepted all the time (3.7 behaviour)

Actual behavior:
With 3.8 any extra data-* will be rejected if the props type is based on a union.

Related Issues:
In another repo, but may be this mui/material-ui#19536 ?

Code

import React, { ReactElement} from "react";

declare function NotHappy(props: ({ fixed?: boolean } | { value?: number })): ReactElement;
declare function Happy(props: { fixed?: boolean, value?: number }): ReactElement;

const RootNotHappy = () => (<NotHappy data-testid="my-test-id" />)
const RootHappy = () => (<Happy data-testid="my-test-id" />)
Compiler Options
{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "useDefineForClassFields": false,
    "alwaysStrict": true,
    "allowUnreachableCode": false,
    "allowUnusedLabels": false,
    "downlevelIteration": false,
    "noEmitHelpers": false,
    "noLib": false,
    "noStrictGenericChecks": false,
    "noUnusedLocals": false,
    "noUnusedParameters": false,
    "esModuleInterop": true,
    "preserveConstEnums": false,
    "removeComments": false,
    "skipLibCheck": false,
    "checkJs": false,
    "allowJs": false,
    "declaration": true,
    "experimentalDecorators": false,
    "emitDecoratorMetadata": false,
    "target": "Latest",
    "jsx": "Preserve",
    "module": "ESNext"
  }
}

Playground Link: Provided

@Retsam
Copy link

Retsam commented Feb 21, 2020

As a short-term workaround:

<NotHappy data-testid={"my-test-id" as never} />

But I agree, this looks like it should work to me.

@orta
Copy link
Contributor

orta commented Feb 23, 2020

I've heard two reports of this so far, might be worth including in a 3.8 patch also

@jraoult
Copy link
Author

jraoult commented Feb 23, 2020

@orta it would be great. If the fix it is not making it in a patch release for the 3.8 we will have to skip the whole minor. I can imagine that we are not the only ones in this case. This is not uncommon for people to use data-* in React, especially peeps using Cypress where it is even considered a best practice.

@chandlerprall
Copy link

chandlerprall commented Feb 24, 2020

I've heard two reports of this so far, might be worth including in a 3.8 patch also

This affects Elastic UI as well; similar to Material UI we have a number of components whose props are unions. If this break is not fixed in 3.8 it not only affects our Elastic&Material UI projects directly but also blocks any downstream consumers from using TypeScript 3.8 + these component libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants