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

New hot API #1122

Merged
merged 18 commits into from
Dec 13, 2018
Merged

New hot API #1122

merged 18 commits into from
Dec 13, 2018

Conversation

theKashey
Copy link
Collaborator

This PR is aimed to solve 2 issues:

The change

  • split hot into two pieces
import {hot} from 'react-hot-loader/root';
//^^ this will setup HMR for the "parent"(current) module, and then wipe itself from a cache, to execute the same action for the next call

export hot(Component);
// ^ this call is a bit shorter, and does not use magic `module` variable.

If "first" part is called, but second is not - then some JS error took a place, and we should display an Error overlay.

@codecov-io
Copy link

codecov-io commented Dec 11, 2018

Codecov Report

Merging #1122 into master will decrease coverage by 0.83%.
The diff coverage is 64.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1122      +/-   ##
==========================================
- Coverage   84.89%   84.05%   -0.84%     
==========================================
  Files          33       37       +4     
  Lines        1026     1154     +128     
  Branches      258      281      +23     
==========================================
+ Hits          871      970      +99     
- Misses        121      143      +22     
- Partials       34       41       +7
Impacted Files Coverage Δ
src/utils.dev.js 92.3% <ø> (ø) ⬆️
src/configuration.js 100% <100%> (ø) ⬆️
src/proxy/createClassProxy.js 97.48% <100%> (+0.06%) ⬆️
src/global/modules.js 100% <100%> (ø) ⬆️
src/index.dev.js 100% <100%> (ø) ⬆️
src/global/generation.js 100% <100%> (ø) ⬆️
src/reconciler/hotReplacementRender.js 86.95% <100%> (ø) ⬆️
src/AppContainer.dev.js 88.46% <100%> (+7.5%) ⬆️
src/reactHotLoader.js 81.66% <33.33%> (+24.88%) ⬆️
src/reconciler/fiberUpdater.js 35.29% <35.29%> (ø)
... and 11 more

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 ef001dc...ca92718. Read the comment docs.

@theKashey
Copy link
Collaborator Author

Next step - on hotUpdateEnter amend every mounted (tracked in createProxy) component with componentDidCatch, and remove all clean components on hotUpdateLeave, keeping only dirty ones with ErrorOverlay displayed in place.
That would match principles

@gregberge
Copy link
Collaborator

I think it is very nice to simplify API and to remove the magic "module" part! Good job!
I can't review it in detail because I no longer understand the whole code.

@theKashey
Copy link
Collaborator Author

@neoziro - you know - I do have the same problem. Sometimes I am thinking to start everything, especially tests, from scratch.

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.

None yet

3 participants