react-live icon indicating copy to clipboard operation
react-live copied to clipboard

typescript support

Open libetl opened this issue 2 years ago • 4 comments

Use language option (when set to either typescript or tsx) to change the sucrase transform options. image

libetl avatar Mar 10 '22 10:03 libetl

@jpdriver 👋🏻 Hello

libetl avatar Mar 10 '22 10:03 libetl

Would love to see this get merged. @jpdriver any updates?

gutenye avatar Apr 26 '22 04:04 gutenye

TypeScript support is definitely something that's on my mind -- but I'm afraid unfortunately it isn't as straight-forward as we'd like it to be.

I really appreciate the intent behind this PR, but currently code like this

class Test extends React.Component<{ name: string}> {
  render() {
    return <div>Hello {this.props.name}!</div>
  }
};

will compile and render correctly while code like this

interface Props {
  name: string
}

class Test extends React.Component<Props> {
  render() {
    return <div>Hello {this.props.name}!</div>
  }
};

will throw an exception.

the issue here is that at the moment the code we pass into Sucrase is already wrapped in a return() and ready to be eval'd.

  • see https://github.com/FormidableLabs/react-live/blob/master/src/utils/transpile/index.js#L11

to make this work in all cases, we'd have to swap the order of operations around a bit and perform the transpilation step (so Sucrase can strip out the interface in the example above) earlier.

i'm definitely all for adding support for TypeScript down the road, but it needs to be flexible enough to the point where both snippets above work the same way.

the other tricky thing is ensuring that no existing code or examples are transformed differently; which is not something i've been able to do just yet.

so this may need to be put off until the next major release -- tbc 🔮

jpdriver avatar Apr 27 '22 23:04 jpdriver

I understand, thanks for the follow up and for taking some time to answer.

libetl avatar Apr 28 '22 13:04 libetl

@jpdriver What's the downside in allowing users to customize the sucrase config themselves? Then you won't get any breaking changes. I've never had problem using typescript itself to transform code transformCode, so I'm curious why simply having sucrase run in that order of operations wouldn't work. Are you trying to run it after your own code is added? If so - why?

ntucker avatar Mar 24 '23 02:03 ntucker

We will look at more in-depth TypeScript integration in the next major release version and consider the trade-offs between exposing the configuration of surcrase.

carloskelly13 avatar Mar 24 '23 13:03 carloskelly13

[email protected] now supports TypeScript by default. Please check it out and let us know any feedback. Thanks!

carloskelly13 avatar Mar 30 '23 12:03 carloskelly13

Thanks @carloskelly13 ! This is a huge win; though I'd still love to customize more of sucrase if possible.

ntucker avatar Mar 30 '23 16:03 ntucker

@ntucker Exposing sucrase options has some complexities it's not as simple as just passing typescript as a transformer option. We have pull things apart when we have custom defined types or other non "renderable" things for implicit returns. So we'd have to see which pass of sucrase for the implicit returns components we'd want to pass options to, perhaps both. Always open to ideas.

carloskelly13 avatar Mar 31 '23 23:03 carloskelly13

@carloskelly13 I only use it with noInline because I need multiple functions in one example. Because of that my code goes through the simpler path: https://github.com/FormidableLabs/react-live/pull/344/files#diff-8024f5483f023d9a9373678ffd2c1d68cdabde833d00e5d64ff9ba27baf6f0d3R60

Personally I would much prefer it it looked for a export default to determine which function to render.

Because I don't use it...maybe there's something missing about the other transform, but for https://github.com/FormidableLabs/react-live/pull/344/files#diff-8024f5483f023d9a9373678ffd2c1d68cdabde833d00e5d64ff9ba27baf6f0d3R24 can't you just use the options a user provides for the first transform and only do transform({ transforms: ["imports"] }) like you were doing later? If a user specified 'import' transform will break this you could always just remove it from the user list since you know it will be done after.

ntucker avatar Mar 31 '23 23:03 ntucker