Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify TypeScript configuration #996

Closed
wants to merge 1 commit into from

Conversation

lostfictions
Copy link
Contributor

As I tried to figure out how to upgrade to React Hot Loader 4 today, I was a bit confused by the options for configuring it for use with TypeScript -- it seems less than ideal to chain loaders, and in particular requiring folks to change their tsconfig is pretty annoying. Either way, there's a performance hit.

I think the ideal, most non-disruptive solution for TypeScript users would be for RHL to eventually support a TypeScript custom transform plugin as discussed here -- but for now, since Babel can consume TypeScript syntax and transpile it, why not recommend using it alone as the simplest method? There's no requirement for users to change their tsconfig, rebuilds are lightning-fast, and you can still typecheck with fork-ts-checker-webpack-plugin (or manually invoking tsc --noEmit).

In this PR I've provisionally updated the README with more verbose (but hopefully simpler) instructions and changed the TypeScript example to reflect this configuration -- maybe folks can try it out and see what they think?

@codecov-io
Copy link

codecov-io commented May 29, 2018

Codecov Report

Merging #996 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #996   +/-   ##
=======================================
  Coverage   86.65%   86.65%           
=======================================
  Files          31       31           
  Lines         802      802           
  Branches      189      189           
=======================================
  Hits          695      695           
  Misses         89       89           
  Partials       18       18

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ec2521...a30db26. Read the comment docs.

@theKashey
Copy link
Collaborator

I think - its time to create a separate TS documentation file, with more deeper explanations, as long - we have got tooooo many variants.

@micimize
Copy link

the preset here depends on @babel/plugin-transform-typescript, which among other things doesn't support namespaces. I think the syntax-typescript the previous babel-only setup used has the same problem (they don't say explicitly).

For some people the performance is worth the trade-off, but it should be noted. I personally wasn't able to get a functioning typescript-first config with my environment, I think because of allowSyntheticDefaultImports or some other setting.

@theKashey
Copy link
Collaborator

allowSyntheticDefaultImports does not change the way code got generated.
I thing we have to make a pause, and outline all possible way to handle TS, more examples to test, and all caveats found.

@theKashey
Copy link
Collaborator

merged as a part of another PR

@theKashey theKashey closed this Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants