Skip to content

[typescript] Fix keyof for typescript@2.9#11669

Merged
pelotom merged 2 commits intomui:masterfrom
mctep:fix-keyof
Jun 1, 2018
Merged

[typescript] Fix keyof for typescript@2.9#11669
pelotom merged 2 commits intomui:masterfrom
mctep:fix-keyof

Conversation

@mctep
Copy link
Copy Markdown
Contributor

@mctep mctep commented Jun 1, 2018

Fixes #11656

@mctep mctep changed the title fix keyof for ts 2.9 Fix keyof for typescript@2.9 Jun 1, 2018
@oliviertassinari oliviertassinari requested a review from pelotom June 1, 2018 16:43
* certain `classes`, on which one can also set a top-level `className` and inline
* `style`.
*/
export type StandardProps<C, ClassKey extends string, Removals extends keyof C = never> = Omit<
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not just redefine Omit to allow using any valid key type?

export type Omit<T, K extends keyof any> = Pick<T, Exclude<keyof T, K>>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. It is better. Thank you! 👍

@pelotom pelotom merged commit 2a86083 into mui:master Jun 1, 2018
@pelotom
Copy link
Copy Markdown
Collaborator

pelotom commented Jun 1, 2018

Thanks!

@oliviertassinari oliviertassinari changed the title Fix keyof for typescript@2.9 [typescript] Fix keyof for typescript@2.9 Jun 3, 2018
@activebiz
Copy link
Copy Markdown

I am still having the same issue in "@material-ui/core": "1.2.0","@material-ui/icons": "1.1.0",. Not sure if I am not pulling right version?

@mctep
Copy link
Copy Markdown
Contributor Author

mctep commented Jun 4, 2018

@activebiz Thats right. Could you also provide your TS version, tsconfig.json and error message?

@activebiz
Copy link
Copy Markdown

activebiz commented Jun 4, 2018

@mctep thanks
TS is 2.9.1
I get this same issue:
#11671

If I revert the TS to 2.8.4 I get following:

` node_modules/@material-ui/core/ButtonBase/ButtonBase.d.ts
(11,38): Namespace 'React' has no exported member 'RefObject'.

node_modules/@material-ui/core/styles/withStyles.d.ts (51,37): Namespace 'React' has no exported member 'RefObject'.

Thanks

@mctep
Copy link
Copy Markdown
Contributor Author

mctep commented Jun 4, 2018

@activebiz Looks like that it is not the same issues. Which versions of @types/react and @types/react-dom do you use? Try to update it to the latest.

@Ritorna
Copy link
Copy Markdown
Contributor

Ritorna commented Jun 5, 2018

Same as in issue #11671 here.
Using TypeScript 2.9.1, React 16.4. @types/react 16.3.16 and @Material-ui/core 1.2.0

@activebiz
Copy link
Copy Markdown

@mctep exactly what @Ritorna said

Typescript 2.9.1
React 16.4
React Typescript def: 16.3.16
Material ui core typescript def: 1.2.0

@Ritorna
Copy link
Copy Markdown
Contributor

Ritorna commented Jun 5, 2018

@mctep, @activebiz see issue #11698
Worked for me there, just set "allowSyntheticDefaultImports": false in your config, or use the latest Typescript build.

@activebiz
Copy link
Copy Markdown

activebiz commented Jun 5, 2018

thanks @Ritorna. Now those errors are gone but got hundreds of new errors. Looks like something wrong with my tsconfig. would you mind sharing your tsconfig ?

Here's mine

{
  "compilerOptions": {
     ...
    "experimentalDecorators": true,
    "allowSyntheticDefaultImports": false,
    "sourceMap": true,
    "moduleResolution": "node",
    "strictNullChecks": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "types": [
      "react-dom",
      "react",
      "node"
    ],
    "lib": ["es2015", "es2017", "dom"]
  },
...
}

@Ritorna
Copy link
Copy Markdown
Contributor

Ritorna commented Jun 5, 2018

@activebiz i guess it mainly depends on which modules you import :/

Here you are:
https://gist.github.com/Ritorna/56885575b7241688af6aca7375302e18.js

Please see this:
image

Alternatively you may use typescript@next preview, this also fixed it for me

@activebiz
Copy link
Copy Markdown

For me following fixed all the d.ts erros.

image

@michael-land
Copy link
Copy Markdown
Contributor

Looks like it cannot correctly infer type def when export component with compose helper.

// Example.tsx
const Example: React.SFC<WithStyles<typeof styles>> = (props) => {
  const { classes } = props;

  return <div className={classes.root}>Material-UI</div>;
};

const styles = ({  }: Theme) =>
  createStyles({
    root: {},
  });

export default compose(
  withStyles(styles),
  withTheme()
)(Example);

// Usage.jsx
import React from 'react';

import Example from './Example';
const Usage: React.SFC = () => <Example />;   // [ts] JSX element type 'Example' does not have any construct or call signatures.

It will works if you just export default withTheme()(withStyles(styles)(Example));

It also doesn't work with other HOC such as redux connect() utility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants