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

Fixes #913 - Fix/improve guide for Electron #994

Merged
merged 4 commits into from
Jun 5, 2018

Conversation

s-h-a-d-o-w
Copy link
Contributor

Apologies for the delay. I wanted to see how the situation with electron-compile's HMR would develop before committing to a recommendation.

And I had to come up with a good workflow myself. Because it was only after I submitted issue #913 that I had the fairly banal realization that one could simply view an Electron app like any other web app. And that maybe one should. Most people probably choose to use Electron because they are already familiar with JS. So they've probably already used webpack. Why introduce new tools that at least partly don't work as well as webpack does?

But I left the reference to electron-compile in there because it is quite popular, it does require less configuration and some people have argued that webpack should not be used with Electron. (Although they possibly overlook the fact that parse time may still play a role even when a big project is loaded directly from one's hard drive...)

Apologies for the delay. I wanted to see how the situation with electron-compile's HMR would develop before committing to a recommendation.

And I had to come up with a good workflow myself. Because it was only after I submitted issue gaearon#913 that I had the fairly banal realization that one could simply view an Electron app like any other web app. And that maybe one should. Most people probably choose to use Electron because they are already familiar with JS. So they've probably already used webpack. Why introduce new tools that at least partly don't work as well as webpack does?

But I left the reference to electron-compile in there because it is quite popular, it does require less configuration and some people have argued that webpack should not be used with Electron. (Although they possibly overlook the fact that parse time may still play a role even when a big project is loaded directly from one's hard drive...)
@codecov-io
Copy link

codecov-io commented May 28, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #994   +/-   ##
=======================================
  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...32ccc09. Read the comment docs.

Copy link
Collaborator

@gregberge gregberge left a comment

Choose a reason for hiding this comment

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

My review is strictly typographical as I have never used Electron + React Hot Loader!

README.md Outdated
```js
enableLiveReload({ strategy: 'react-hmr' })
```
One way of doing this with Electron is to simply use webpack like any web-based project might do and the general guide above describes. See also [this example Electron app](https://github.com/s-h-a-d-o-w/rhl-electron-quick-start)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Webpack*

README.md Outdated

See a [complete example](https://github.com/rllola/hmr-example-issue-2/blob/master/src/index.js).
A webpack-less way of doing it to use `electron-compile` (which is also used by `electron-forge`) - see [this example](https://github.com/rllola/hmr-example-issue-2). While it requires less configuration, something to keep in mind is that `electron-compile`'s HMR will always reload all modules, regardless of what was actually edited.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Webpack-less*

  • Links on electron-compile and electron-forge

@s-h-a-d-o-w
Copy link
Contributor Author

Hm... I didn't see that and you're obviously right for consistency's sake.

However, I would like to point out that the official way of spelling "webpack" is indeed with a lower-case w in the beginning: https://webpack.js.org/concepts/
So it seems to me that instead, all occurrences of "Webpack" in the rest of the README should be changed.

@gregberge
Copy link
Collaborator

@s-h-a-d-o-w you are right, let's go for "webpack"!

@theKashey
Copy link
Collaborator

Never too late 👍

@theKashey
Copy link
Collaborator

As far webpack were "right" - that we are waiting for?
Links to electron forge?

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