nukeop / nuclear

Streaming music player that finds free music for you
https://nuclear.js.org/
GNU Affero General Public License v3.0
11.93k stars 1.03k forks source link

Hot-reloading is broken #692

Open Venryx opened 4 years ago

Venryx commented 4 years ago

Hot reloading does not appear to be working currently.

If changes are made to, eg. LibraryView/index.js, I get this error when the electron window tries to load in the changes:

App.js:137 Uncaught TypeError: this.props.actions.createPlugins is not a function
    at App.componentDidMount (VM2904 App.js:137)
    at App.componentDidMount (react-hot-loader.development.js:704)
    at commitLifeCycles (react-dom.development.js:22068)
    at commitLayoutEffects (react-dom.development.js:25302)
    at HTMLUnknownElement.callCallback (react-dom.development.js:336)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:385)
    at invokeGuardedCallback (react-dom.development.js:440)
    at commitRootImpl (react-dom.development.js:25040)
    at unstable_runWithPriority (scheduler.development.js:818)
    at runWithPriority$2 (react-dom.development.js:12130)
    at commitRoot (react-dom.development.js:24889)
    at finishSyncRender (react-dom.development.js:24296)
    at performSyncWorkOnRoot (react-dom.development.js:24274)
    at eval (react-dom.development.js:12180)
    at unstable_runWithPriority (scheduler.development.js:818)
    at runWithPriority$2 (react-dom.development.js:12130)
componentDidMount @ App.js:137
componentDidMount @ react-hot-loader.development.js:704
commitLifeCycles @ react-dom.development.js:22068
commitLayoutEffects @ react-dom.development.js:25302
callCallback @ react-dom.development.js:336
invokeGuardedCallbackDev @ react-dom.development.js:385
invokeGuardedCallback @ react-dom.development.js:440
commitRootImpl @ react-dom.development.js:25040
unstable_runWithPriority @ scheduler.development.js:818
runWithPriority$2 @ react-dom.development.js:12130
commitRoot @ react-dom.development.js:24889
finishSyncRender @ react-dom.development.js:24296
performSyncWorkOnRoot @ react-dom.development.js:24274
(anonymous) @ react-dom.development.js:12180
unstable_runWithPriority @ scheduler.development.js:818
runWithPriority$2 @ react-dom.development.js:12130
flushSyncCallbackQueueImpl @ react-dom.development.js:12175
flushSyncCallbackQueue @ react-dom.development.js:12163
scheduleUpdateOnFiber @ react-dom.development.js:23676
updateContainer @ react-dom.development.js:27061
legacyRenderSubtreeIntoContainer @ react-dom.development.js:27501
render @ react-dom.development.js:27572
render @ index.js:57
async function (async)
render @ index.js:48
(anonymous) @ index.js:64
./app/index.js @ renderer.js:48818
__webpack_require__ @ renderer.js:727
hotApply @ renderer.js:660
(anonymous) @ renderer.js:314
Promise.then (async)
hotUpdateDownloaded @ renderer.js:313
hotAddUpdateChunk @ renderer.js:289
webpackHotUpdateCallback @ renderer.js:8
(anonymous) @ main.8d44e9f9eeb6d63b3bf6.hot-update.js:1

Here is the line of code that errors: https://github.com/nukeop/nuclear/blob/ed0320465ff1489e21bc1a2ced17a529884ea51c/packages/app/app/App.js#L73

To reproduce, just start the npm run start script, wait for the app to fully start, add some meaningless line to the render function, then watch as the electron window tries to hot-reload but hits the error above.

That this hasn't been resolved yet makes me think either that: 1) I'm not starting the development watch process correctly. (though I'd expect npm run start to be sufficient...) 2) There have been changes recently to master that broke things and needs finishing. 3) There's some sort of cache or something that I need to clear, to have the current codebase's hot-reload function properly. (clear the redux store? clear a data folder somewhere? etc.)

Any thoughts?

nukeop commented 4 years ago

I think this is a problem with a missing prop in LibraryView, not necessarily with hot reload itself.

Venryx commented 4 years ago

I think this is a problem with a missing prop in LibraryView, not necessarily with hot reload itself.

But: 1) The error only occurs during the hot-reload; if I refresh the page, my changes show up without any error. 2) The error occurs in the componentDidMount function of App. (not in LibraryView itself) 3) The error shows up regardless of what file I make the change in, so long as its within the app package. (the LibraryView file was just an example of a file you can make a random change in to trigger the hot-reload)

For narrowing-down purposes: Does hot-reloading work for you? (Or do you not really use it?)

nukeop commented 4 years ago

I have the same problem as you, but not in all cases. E.g. css changes are always applied cleanly.

Hot reload problems are sadly pervasive in large React codebases, nearly every project I worked with that reached a certain size had at least some problems with hot reload.

Venryx commented 4 years ago

Hot reload problems are sadly pervasive in large React codebases, nearly every project I worked with that reached a certain size had at least some problems with hot reload.

Same. I used it at first in my projects, but kept hitting various issues, so eventually gave up and just refresh the page after each change now. (I use webpack-dev-server for incremental compilation, and my page loads pretty fast, so it's barely slower than hot-reloading anyway)

It's one of those things that is cool to try out, but at the end of the day, it's both not as stable, or as necessary, as it first seemed.

In my opinion, it's only really helpful if either: 1) The software is badly optimized, such that the page/program startup takes ages. 2) Navigating to the part of the UI you're developing takes a long time. (due to not persisting the UI state thoroughly enough)

Anyway, if hot-reloading is broken in Nuclear anyway, it might be worth disabling it to prevent frustration for new devs; this would also make the refresh process a bit faster, since then the dev-tools wouldn't freeze briefly (from hitting the hot-reload bug), before each manual refresh.

(granted, this would mean we'd lose out on the instant css hot-reloads, so idk... maybe there's a way to have hot-reload only applied for the working css portion?)

nukeop commented 4 years ago

I'm not sure how, but it seems that I fixed it somehow somewhere along the way. When you manage to get it to work, could you please confirm?