Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

chore(docs): make HMR working properly #1370

Merged
merged 10 commits into from
May 26, 2019
Merged

chore(docs): make HMR working properly #1370

merged 10 commits into from
May 26, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 22, 2019

Related #508.

This PR updates:

  • usages of react-hot-loader to use their moden hot() API
  • updates react-source-render, I rewrote it to omit React Context (use only render props) and be compatible with HMR (unstable_hot prop)

Works properly

  • reload of components everywhere
  • reload of example sources in maximized view

Still broken

  • example's source reload on component's pages

image

  • warning about missing Provider
    image

Occurred because there is an issue with lost context: gaearon/react-hot-loader#1207

@DustyTheBot
Copy link
Collaborator

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS

@codecov
Copy link

codecov bot commented May 22, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1370   +/-   ##
=======================================
  Coverage   73.47%   73.47%           
=======================================
  Files         778      778           
  Lines        5859     5859           
  Branches     1726     1726           
=======================================
  Hits         4305     4305           
  Misses       1548     1548           
  Partials        6        6

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 37ebb61...91be85f. Read the comment docs.

<div style={{ paddingBottom: '10px' }} />
{({ element, error, markup }) => (
<>
<Provider.Consumer
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need Provider.Consumer more there

}
})
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stuff will be done by react-hot-loader

import PopupsPrototype from './prototypes/popups'
import IconViewerPrototype from './prototypes/IconViewer'
import MenuButtonPrototype from './prototypes/MenuButton'
import AlertsPrototype from './prototypes/alerts'
Copy link
Member Author

@layershifter layershifter May 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using Webpack 4, these imports will be tree-shaken in production

This change is required: to prevent errors related to HMR like this:
image

// https://github.com/webpack/webpack/issues/834#issuecomment-76590576
module.hot.accept(exampleSourcesContext.id, () => {
exampleSourcesContext = require.context('docs/src/exampleSources/', true, /.source.json$/)
})
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context should be updated separately.

Copy link
Collaborator

@bmdalex bmdalex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 should we organize a session to test the docs to be sure we didn't break something?

@layershifter
Copy link
Member Author

@Bugaa92 at least it doesn't make anything worst 😸 It will affect only local development...

I will merge it and will be able to detect and fix more cases, so I am opened for feedback when more people will be able to perform live test

@layershifter layershifter merged commit f1283e9 into master May 26, 2019
@delete-merged-branch delete-merged-branch bot deleted the chore/enable-hmr branch May 26, 2019 15:53
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants