ds300 / react-native-typescript-transformer

Seamlessly use TypeScript with React Native
MIT License
657 stars 50 forks source link

[Feature request] Inspect noEmitOnError compiler option #15

Closed cbrevik closed 6 years ago

cbrevik commented 7 years ago

Ref https://github.com/ds300/react-native-typescript-transformer/issues/13#issuecomment-310994943

Preferred behaviour would be that the transformer reports errors through packager as normally happens while developing with React Native.

ds300 commented 7 years ago

Sorry for the delay on this. I hope to have time for it on Saturday.

ds300 commented 7 years ago

It just occurred to me that there is a flaw in this plan: changes to a file might produce type errors in another, unchanged file, which won't be reported when transpiling on a module-by-module basis. There's no easy way to solve this that I can see. We'd have to use a more holistic incremental approach like tsc --watch does, but triggering recompiles on-demand from the transformer rather than through watching the file system. But there might be hard-to-resolve issues in doing that, for example: does TS still emit helper functions in every module when compiling incrementally? If not, perhaps a change in one file might result in more than one file being re-emitted, and then what? RN is not set up to handle that kind of situation.

Or maybe we could use the incremental system for error reporting and use standard module transpilation for emitting code like we do now. That would be less performant but it would probably have more straightforward semantics and implementation.

🤔

cbrevik commented 7 years ago

To simplify the feature request, what if it only reports these errors on the initial packager/transformer run?

If it means keeping the the transpilation more performant, which honestly is even more important, I could live with it only catching isolated errors on the initial run. And having to restart the packager in order to do a thorough check again.

Granted, that may be confusing behaviour to some people?

ds300 commented 7 years ago

Yeah I'm not crazy about that idea. I think it would make for poor DX. I'll try to determine how easy it will be to piggy back on tsc's incremental compilation features first, and whether we can (or need to) do anything about the multiple-module-emit situation, and if it looks like it would be too much work maybe then we could consider something more straightforward but less good.

ds300 commented 7 years ago

One thing which is quite straightforward indeed (since it involves me not doing any work :D) is to just run tsc --noEmit --watch yourself. Or is there some special benefit you get from having one/both of:

  1. red screen errors in the app
  2. error reports being emitted on the packager's stdout

Or even something else I'm not identifying?

cbrevik commented 7 years ago

Well, for my specific case, it is because I'm working a bit with devs who are used to statically typed languages like C#. They are used to the app breaking on compilation if something is wrong. And the same is true in cases where they have used TypeScript.

So it's more about keeping a familiar and non-confusing workflow.

The main goal I had in mind, when using TypeScript with RN, was easy onboarding with the app projects. So thats why I prefer making the transformer default via rn-cli.config.js. Less commands to learn, and we don't venture too far away from the usual workflow.

Was hoping the same could be applied to errors as well. Make the app break on errors like they are used to, and make it default behaviour so there's less to remember other than just running the app.

I see your point about just running tsc --noEmit --watch though (which is what I do myself).

Edit: If it's a lot of work to implement, then we can just run tsc instead. Not like running a --watch is that much work, just initially a bit confusing behaviour that adding static typing does not break transpiling on errors. :+1:

cbrevik commented 6 years ago

Just checking back here. What we ended up doing was using husky for precommit and prepush hooks, and running tsc --noEmit in those cases.

Works fine for us, and should be a good workaround. You can close this issue if you want.

ds300 commented 6 years ago

Cool, thanks for the update. Glad you found something that worked for you.

rogerkerse commented 5 years ago

I was googling for hours as well. When using typescript I would have expected yellow and red screens in the app for warnings and errors as well. For example if you create-react-app and create typescript project, then your webpage is covered with error when there is something wrong in code. With this in react-native it swallows all up :(

fantasy525 commented 5 years ago

I was googling for hours as well. When using typescript I would have expected yellow and red screens in the app for warnings and errors as well. For example if you create-react-app and create typescript project, then your webpage is covered with error when there is something wrong in code. With this in react-native it swallows all up :(

Have you found a solution?