-
Notifications
You must be signed in to change notification settings - Fork 194
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
Add react-refresh-typescript to documentation #248
Conversation
Hey - thanks for the PR! I'm open to this change, but I want to further formulate the narrative a bit that this is not officially maintained by the React team (not to disregard your work, but for some people who work in corps or cares about being purely React they would need that information) - can you add that in? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - can you please (if you haven't) test run the example and make sure that it works, and also would recover from errors?
(I'll do it when I merge the PR but just want to be double sure)
I've tried hmr works but not error recovery |
I'll further test this out a bit now that we have all the tests back up 😃 |
hi, any progress? |
Setting up tests to run for this scenario now - if they all pass I'll merge this in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. This is the solution I was looking for.
rebased and resolve reviews |
@pmmmwh Please Review 👍 |
Hi, just dropping in to say that this approach fixed my problem of being unable to use this plugin with TypeScript. My single gripe is that you need to Also, the readme change doesn't seem to mention that CommonJS isn't supported as tsconfig target. |
@yoruvo I wrote no commonjs support in my repo maybe I should add it here too. Nice catch, generally you don't need to feed commonJS to webpack so you can overwrite your settings in your ts-loader |
@yoruvo In 1.1.1 you can require it directly without |
@Jack-Works @pmmmwh Is it ready? |
It's production ready in our app. You can install and use it without this PR, this is just a document PR |
Hi, I'm very sorry that I have been dragging this for such a long time. I have a partially working test setup for this and I really want this to pass all conformance tests before officially recommending it. I'll see if I can whip up a branch to track the status of that this week. |
close #211, close #34