strothj / react-app-rewire-typescript-babel-preset

Add TypeScript support to Create React App using @babel/preset-typescript
https://react-app-rewire-typescript-babel-preset.netlify.com
MIT License
52 stars 6 forks source link

Give *ts* extensions higher precedence than existing extensions #5

Closed detrohutt closed 6 years ago

detrohutt commented 6 years ago

I have a somewhat complex project structure involving a Lerna mono-repo with isomorphic packages using ES Modules ("target": "esnext" and "module": "none" in tsconfig.json & running with node --experimental-modules index.js after building). I'm just now getting to a point where I'm ready to build for production for the first time.

If I build CRA first everything is cool, but if I try to build it after building my isomorphic packages and server, the CRA build tries to use the precompiled .js files, compiles with a bunch of warnings, and then breaks at runtime.

I could make clean scripts to remove the .js files everywhere and each time I build run those first, then build CRA, then build everything else. It'd be much nicer though if CRA would just use the original .ts files, even with the existence of some .js files. That's what this PR accomplishes. I don't know what other effects this may have in other workflows, but it works for my use case so I figured I'd present it for your consideration. :)

strothj commented 6 years ago

Thanks again! Published as version 2.1.1.

detrohutt commented 6 years ago

Thanks for the fast response time as usual. After looking at the code for babel-plugin-transform-typescript, I realized it doesn't actually take your typescript settings into account or compile the code at all. It literally just strips the type annotations. My code apparently works with CRA without needing any compilation, but I'm afraid others who aren't targeting esnext will need to compile their code to .js before building CRA and would therefore be expecting and relying on the behavior I just removed. They'd have the opposite problem I had and would have to either remove their .ts files (obviously not ok) or build to a different location and set up their imports to look there through webpack or some other means... I could be wrong about this but that's my best guess, so it's probably best to revert this unless I'm misunderstanding something about how all this works. Sorry for the trouble. :disappointed:

detrohutt commented 6 years ago

Hmm.. just had a thought. Would it be possible to make this into an option? Then it could be set to the previous behavior by default and I could just toggle the option in my case.

detrohutt commented 6 years ago

I looked at the docs for react-app-rewire again and found this:

Some change with rewire, if you want to add some extra param for rewire

Optional params:
you can see react-app-rewire-less

So I checked out react-app-rewire-less and it doesn't look very complicated and wouldn't be a breaking change (unlike my last commit lol). If you want to wait a bit I'll see if I can make a PR to make it into an option so you can just bump to 2.2.0 instead of reverting.