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

Hot Module Replacement Documentation #1251

Merged
merged 8 commits into from
Jun 3, 2017
Merged

Conversation

skipjack
Copy link
Collaborator

@skipjack skipjack commented May 27, 2017

The goal of this PR is to improve the HMR docs in variety of ways:

  • Add a simpler HMR article that starts from scratch (maybe should be moved to /guides?)
  • Add documentation for the HotModuleReplacementPlugin
  • Potentially replace or at least simplify the hmr-react guide
  • Start documenting the module (e.g. module.hot.accept) and node APIs
  • Link any current/new HMR docs together in an intuitive way
  • Fill in option descriptions in /plugins/hot-module-replacement-plugin

Resolves #86
Resolves #88 (see comment below)

@skipjack
Copy link
Collaborator Author

skipjack commented May 27, 2017

@webpack/documentation-team my feeling, and based off some of the issues/comments I've seen around this topic, is that the current hmr concepts page is useful but the hmr-react guide is too broad in scope and overlaps significantly with react-hot-loader's own documentation. I think the more we can document core behaviors and link to external third-party integrations/tools, instead of trying to document them as well, the better off we'll be.

For example, in this case, if react-hot-loader's api or setup process changes (which is very possible since it's in beta) that's more documentation that we have to change here. Add this to the fact that we're still trying to finish documenting our backlog of core issues and I think we're just making things harder on ourselves. At most, I think we should include react-hot-loader's readme dynamically under /loaders as we've done with other community built tools.

@skipjack
Copy link
Collaborator Author

Also can anyone tell me, or fill in the documentation for the multiStep and fullBuildTimeout options in the HotModuleReplacementPlugin? I was able to figure out these options were accepted by reading the source but couldn't figure out exactly what their purposes are.

@skipjack
Copy link
Collaborator Author

@bebraw @simon04 can one of you take a look at this when you have a chance and let me know what's missing? Especially in regards to the actual API bits like module.hot in the module-variables guide and anything in relation to the Node API for HMR which I essentially know nothing about. If there's existing docs or articles out there re the HMR Node API, I'd be happy to port them.

@skipjack
Copy link
Collaborator Author

Ok I found this page in the old docs which covers what is in module.hot but nothing on the NodeJS webpack process management side? Is there any interface besides including the plugin and notifying webpack-dev-server?

@jsf-clabot
Copy link

jsf-clabot commented May 31, 2017

CLA assistant check
All committers have signed the CLA.

@skipjack
Copy link
Collaborator Author

@bebraw instead of copying that documentation for redux hmr I linked to the section in your article. This goes with the same theme of instead of trying to document every framework's hmr setup, linking to that documentation externally. Please review when you have a chance, this should be good to go now aside from the hmr plugin options (which, if you know what they do, please share).

@bebraw
Copy link
Contributor

bebraw commented Jun 1, 2017

@skipjack Yeah, it makes sense to point to third parties with Redux/React if there are docs. I expect Redux documentation might have something on HMR.

@skipjack
Copy link
Collaborator Author

skipjack commented Jun 1, 2017

@bebraw have you encountered this yarn error before?

@bebraw
Copy link
Contributor

bebraw commented Jun 1, 2017

@skipjack Nope. Looks like a serious yarn issue. Check out their issue tracker. Others might have the same problem.

@skipjack
Copy link
Collaborator Author

skipjack commented Jun 2, 2017

Ok will do -- are you familiar with those plugin options by chance? See the last task above:

Fill in option descriptions in /plugins/hot-module-replacement-plugin

And then aside from that are you good with this being merged?

@bebraw
Copy link
Contributor

bebraw commented Jun 2, 2017

multiStep has to do with performance. Enabling it is a good idea on larger projects but if I remember right, it's broken with html-webpack-plugin. You have to check fullBuildTimeout from the source. I don't remember that one.

Apart from those and the yarn issue this is good to go.

There are now three core pages to document hot-module-replacement (one in
`/concepts`, `/api`, and `/guides`) as well as a page to document the plugin. This
should create a better flow for readers as they can go from `concept` => `api` =>
`guide` to get a well rounded understanding of this feature.
@skipjack
Copy link
Collaborator Author

skipjack commented Jun 3, 2017

Apart from those and the yarn issue this is good to go.

Seems yarn worked itself out, though I did find that related issue that we should track just in case it happens again. Added the plugin option docs and fixed a few broken links... once the build passes I'll merge :shipit:.

@skipjack
Copy link
Collaborator Author

skipjack commented Jun 3, 2017

The build is failing only for edit links that don't exist for the new pages. Running npm run lint:js && npm run lint:markdown && npm run lint:social && npm run lint:prose was clean so this should be good to go. If anything comes up, I'll fix in a separate pr.

@skipjack skipjack merged commit 41117d6 into master Jun 3, 2017
@skipjack skipjack deleted the hot-module-replacement branch June 3, 2017 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants