dotkom / design-system

Component library, maybe
https://design.online.ntnu.no
MIT License
4 stars 4 forks source link

TypeScript doesn't type-check MDX #113

Open plusk opened 4 years ago

plusk commented 4 years ago

This leads to errors all the time with people using for example disabled="false", which would evaluate to disabled because the string "false" is a truthy value. If TypeScript looks over the .mdx files and makes sure the proper types are used, we avoid this.

FluidSense commented 4 years ago

https://github.com/mdx-js/mdx/issues/255 Prettier seemingly supports mdx, perhaps that's something we can check out. https://github.com/rx-ts/eslint-mdx Or this^

JounQin commented 4 years ago

@plusk eslint-mdx has been enabled in the project, is there anything unexpected?

FluidSense commented 4 years ago

@JounQin The error might be related to any of storybook, eslint and typescript. Currently, we write our project in TypeScript and have defined types on props. Given this component:

interface Example {
  input: string,
}

const SomeComponent = ({ input }: Example) => { ... }
export default SomeComponent;

and the mdx-story containing this jsx

<Preview>
  <Story>
     <SomeComponent input={false} />
  </Story>
</Preview>

We would like for this faulty input type to be noticed. Currently, the code above will be accepted by Storybook, it will be accepted by TypeScript (as it does not check .mdx-files) and it will be accepted by eslint. Added eslint-mdx to our project, but the example above is still accepted.

JounQin commented 4 years ago

Please provide a minimal runnable reproduction, otherwise I can't offer any help.

FluidSense commented 4 years ago

Thanks @JounQin! I'll try to set up a minimal runnable reproduction tomorrow.

FluidSense commented 4 years ago

@JounQin I created this minimal project https://github.com/FluidSense/mdx-cra-ts-loader and eslint-mdx works as intended. I will reword our issue as it is TypeScript which is not type-checking MDX. I see this is an issue at another framework as well. MDX bypasses type-checking as it is compiled to jsx, not tsx. Exactly how to address this in our project is still an issue, but it is not related to eslint.

JounQin commented 4 years ago

Then a custom TypeScript transformer plugin is required AFAIK. And that's the worth for using eslint-mdx here, LOL.