search

electron-userland / electron-webpack

Scripts and configurations to compile Electron applications using webpack
https://webpack.electron.build/
906 stars 171 forks source link

HMR for main process does not work if adding entry to webpack.additions.main.js #303

Open cmeeren opened 5 years ago

cmeeren commented 5 years ago

Based on known good electron-webpack-quick-start (main process HMR works fine from the get-go, app restarts as expected when editing main/index.js):

Add webpack.additions.main.js with the following:

var path = require("path");

module.exports = {
  entry: path.join(__dirname, "src/main/index.js"),
}

Add the following to package.json:

"electronWebpack": {
    "main": {
      "webpackConfig": "webpack.additions.main.js"
    }
  },

Now the app is not restarted when making changes to main/index.js. (The app otherwise works identically.)

I would expect main process HMR to work correctly even though I explicitly specify the entry point as above.

Further information, in case it's relevant

Here is some console output when editing main/index.js without the extra config above (i.e. when everything works correctly):

┏ Main -----------------------

  Hash: 2286fcb35f1542ca30fd
  Version: webpack 4.35.0
  Time: 30ms
  Built at: 2019-07-08 3:49:40 PM
                                    Asset      Size  Chunks             Chunk Names
     701bc5d738c52096d90b.hot-update.json  46 bytes          [emitted]
  main.701bc5d738c52096d90b.hot-update.js  7.05 KiB    main  [emitted]  main
                                  main.js  41.6 KiB    main  [emitted]  main
  Entrypoint main = main.js main.701bc5d738c52096d90b.hot-update.js
  [0] multi ./node_modules/electron-webpack/out/electron-main-hmr/main-hmr ./src/main/index.js 40 bytes {main}
  [./node_modules/electron-webpack/out/electron-main-hmr/main-hmr.js] 582 bytes {main} [built]
  [./src/main/index.js] 1.41 KiB {main} [built]
  [electron] external "electron" 42 bytes {main}
  [electron-webpack/out/electron-main-hmr/HmrClient] external "electron-webpack/out/electron-main-hmr/HmrClient" 42 bytes {main}
  [path] external "path" 42 bytes {main}
  [source-map-support/source-map-support.js] external "source-map-support/source-map-support.js" 42 bytes {main}
  [url] external "url" 42 bytes {main}

┗ ----------------------------
┏ Electron -------------------

  [HMR] Error: Aborted because ./src/main/index.js is not accepted
  Update propagation: ./src/main/index.js -> 0
      at hotApply (C:\Users\Christer\Source\Repos\electron-webpack-quick-start\dist\main\main.js:453:30)
      at C:\Users\Christer\Source\Repos\electron-webpack-quick-start\dist\main\main.js:291:22
  [HMR] Cannot apply update. Need to do a full reload - application will be restarted

┗ ----------------------------

Here is the corresponding output with the configuration above (i.e. when HMR does not work):

┏ Main -----------------------

  Hash: b61034546bbd4573c8db
  Version: webpack 4.35.0
  Time: 30ms
  Built at: 2019-07-08 3:48:47 PM
                                    Asset      Size  Chunks             Chunk Names
     57a1b38f9d575b98ce49.hot-update.json  46 bytes          [emitted]
  main.57a1b38f9d575b98ce49.hot-update.js  7.06 KiB    main  [emitted]  main
                                  main.js  36.3 KiB    main  [emitted]  main
  Entrypoint main = main.js main.57a1b38f9d575b98ce49.hot-update.js
  [./src/main/index.js] 1.41 KiB {main} [built]
  [electron] external "electron" 42 bytes {main}
  [path] external "path" 42 bytes {main}
  [url] external "url" 42 bytes {main}

┗ ----------------------------

I'm on Windows.

loopmode commented 5 years ago

Hi. Unrelated to the actual issue or solution, but did you maybe swap the descriptions on the working and non-working hmr console logs? Looks as if the one that's working claims module was not accepted, while the one that's not working doesn't complain..?

Besides that, I'm reading the source and trying to get a clue about the actual issue/solution. Not sure I can help tho :)

cmeeren commented 5 years ago

No, the logs are correct. I don't know how it works, but I guess the "error" is just HMR's way of telling that the entire app must be restarted (which it subsequently is, as expected).

In the other case, there is no HMR "error" and the app is not restarted, so no changes are visible.

loopmode commented 5 years ago

Okay, so I did some digging, and found some interesting things. I'm on Windows too.

So, first of all, I never cared much about main process HMR - I only needed it for the renderer process in my use case. And I actually never understood the claim of the docs that HMR works for main out of the box. Also I don't understand how it works for you - in case of no custom config. For me, whenever I save main/index.js - or any module required by it - I get a full app restart. And, again, I didn't care much.

So, now that I tried to reproduce your situation, again I was sursprised because it just doesn't work for me without any extras (based on the quick-start main/index.js).

I did some logging here and there, but in the end...I could have thought of the solution without digging...

For me, no matter whether I have a custom config / entry or not, HMR works if I place if (module.hot) module.hot.accept() in main/index.js, and it doesn't work when I do not place it.

I've known this from the renderer, and apparently, it applies to main just the same.

So, just give it a try and accept the hotness! :)

cmeeren commented 5 years ago

For me, whenever I save main/index.js - or any module required by it - I get a full app restart.

AFAIK this is intended, and is what I think is the whole point as far as HMR in the main process goes. It's what I consider "working" above. Because you have to restart the app if you change things that e.g. only occur on startup.

With the non-working config above, there is no app restart, and thus I have to manually quit the app and run yarn dev again.

In my case this takes much longer than simply having electron-webpack/HMR restart the app when the main process code is changed.

Please let me know if I have misunderstood something.

develar commented 5 years ago

https://medium.com/@develar/electron-very-fast-developer-workflow-with-webpack-hmr-e2a2e23590ad

What happens if you don’t implement accept or some error occurred or your main entry file is changed? electron-webpack takes care about it. In this case application will be restarted automatically.

cmeeren commented 5 years ago

Thanks. It seems I have misunderstood things.

Still, I find the behaviour described in this issue confusing.

loopmode commented 5 years ago

I find we need to add the accept part to the docs. Or maybe it's there and I didn't read property.

loopmode commented 5 years ago

Added info about HMR usage to the docs, will be published with next release: https://github.com/electron-userland/electron-webpack/commit/5c9673e6dd5544197c6f5f41f51d89c5c06ab28e

I think we clarified that HMR for main process works, even with custom config/entry. Feel free to reopen if necessary.

cmeeren commented 5 years ago

Thanks, but I don't see how that commit addresses this issue. This issue is about how adding a webpack config for the main process with an entry (even if just set to the file that is used by default) changes the behavior and causes the app not to restart. Why does that happen? Is it intended, or a bug?

loopmode commented 5 years ago

Well, i don't want to be the naive "works for me" type of guy, but... I wasn't able to reproduce what you described, at all.

Regardless of custom entry or not, HMR doesn't just work out of the box. Thus, I can't see a difference in behaviour using the standard setup - no HMR without custom entry, no HMR with custom entry.

However, once I add if (module.hot) module.hot.accept() to main/index.js, HMR works perfectly fine - and again regardless of whether a custom entry is provided or not.

Effectively, I closed the issue because it's not reproducible in a default setup, and because in such default setup it's perfectly possible to have HMR for the main process, both with a custom entry and without.

If you can provide a minimalistic repo where the difference is reproducible, I'll definitely look into it. It would be an interesting edge case after all.

Also if you like, I can provide such a repo with working HMR and custom entry. (However, not off my phone, so earliest tomorrow :))

cmeeren commented 5 years ago

I apologize if I have used HMR incorrectly in this discussion. I am new to JS/Webpack/Electron etc. and HMR might not be the core of the matter here.

I have created a repro, please see my fork: cmeeren/electron-webpack-quick-start

The master branch is identical to the original, and the hmr-repro branch has the simple additions described in the OP.

Run yarn and yarn dev in each of them.

For the master branch, if you edit src/main/index.js, the app restarts.

For the hmr-repro branch, if you perform a similar edit, the app does not restart.

What I don't understand is why there's a difference in behaviour, and how to make the app restart also when specifying entry like described.

For the record, the edit I make to src/main/index.js when testing is simply to remove and re-add

if (isDevelopment) {
  window.webContents.openDevTools()
}
loopmode commented 5 years ago

Hi @cmeeren I too apologize for being somewhat rash. But we're getting closer to mutual understanding here :)

So, first a note about what I looked at, and what HMR actually "means" in this context: It's "kind of good" that the app doesn't restart, that's the idea. (But we do have an issue there nonetheless!)

This is what I tested:

And tadaa! The changed module was "hot-replaced", right in place, without the rest needing to be restarted or replaced in any way. All state your app might have is preserved, only the code of the changed module was replaced in the runtime.

So, that's how it should be!

However, in your repo, this only works a) with if (module.hot) module.hot.accept() and b) when there is no custom entry - just like you said! So.. I'm baffled. I was running the basically same setup when I tested it, and when it worked for me. Unfortunately, that was on a different computer that I cannot access for now.

So, we can resume that yes, there is a difference in behaviour with and without custom entry, and it needs to be investigated.

Also, I need to tweak the module.hot.accept() part to actually do perform a full restart in case the changed file was the entry itself - main/index.js - or you won't be able to make changes to the root file, regardless of whether you have HMR for other imported modules.

Today and tomorrow, I'm on vacation and will be gone, but I'll get back to this asap. Apologies again, we're back at zero :)

cmeeren commented 5 years ago

Any progress on this? :)

ilyabatch commented 5 years ago

Hi @loopmode! Thanks for the explanation, really helped to grasp the concept better. As you mentioned:

Also, I need to tweak the module.hot.accept() part to actually do perform a full restart in case the changed file was the entry itself - main/index.js - or you won't be able to make changes to the root file, regardless of whether you have HMR for other imported modules.

That would be very convenient and frankly the expected default behavior, I'd imagine. Can we anticipate this minor improvement any time soon?

loopmode commented 5 years ago

Not sure something can be done on library side, think we need how to properly do it on userland side..

cmeeren commented 5 years ago

Not sure something can be done on library side, think we need how to properly do it on userland side..

What does this mean? That it must be configured explicitly by all users of electron-webpack? If so, how? (electron-webpack should probably add this to the docs)

loopmode commented 5 years ago

Yes, I believe it's something we as users must do inside the if (module.hot) condition, and we need to figure out what it is. Basically something along the lines of "accept hot change if it was in a child of main/index.js, but do not accept if the change was in main/index.js directly".