systemjs-hmr icon indicating copy to clipboard operation
systemjs-hmr copied to clipboard

Prevent reloading dependants

Open jjrv opened this issue 7 years ago • 7 comments

I didn't find a mechanism to prevent reloading dependant modules all the way up to the root. It's usually a good idea if the code wasn't specifically designed for hot reloading, but I've got at least two cases where the reload shouldn't propagate further:

  • CSS files. Stylesheet updates triggering reloads of the importing JavaScript code seems rarely necessary, but still needed in rare cases (code reading the stylesheet directly or reacting to the computed style).
  • Flux-style one-way data flow frameworks designed for hot reloading. For example, updating a Vue.js component doesn't normally require reloading files that use it. Vue itself handles any necessary reinitialization.

Is there a way to prevent the propagation? If not, I'd suggest two possible mechanisms for consideration:

  • Method or options object importable from @hot to configure reload behaviour.
  • Meta option passed to System.config, allowing to toggle hot reload and its propagation through SystemJS configuration mechanisms per-package, per-extension, per-file etc.

Basically we could control for each source file:

  • Reload when the file changes?
  • Reload when an imported file changes?
  • Pass the reload event to dependants which then reload (unless they disable it through the previous option)?

jjrv avatar Nov 10 '16 12:11 jjrv

I'd be happy to work on a PR in the idea is accepted.

jjrv avatar Nov 10 '16 12:11 jjrv

Sorry that I haven't responded promptly. Been working to meet a few deadlines. Will check this and your other PR out tomorrow. 👍 Glad to see you taking such an interest in the project :)

alexisvincent avatar Nov 10 '16 14:11 alexisvincent

For a quick initial look, I can't think of a reason not to implement this. With regards to config. ATM, the config mechanism for reloads is handled as a second arg to System.reload(moduleName, meta). Where you can specify information regarding how to do the reload. Although it makes sense to also have a config object for the reloader. (Although for now, we can cover all use cases through the meta object). Since System.reload is expected to be called by external tooling it's not too big of an issue at this point in time that config is specified every reload.

Will think about this properly tomorrow.

alexisvincent avatar Nov 10 '16 14:11 alexisvincent

I would like this as well, for css files for example (want to hot reload the stylesheet only, not rerender the whole page from scratch etc.)

srolel avatar Dec 14 '16 00:12 srolel

An easy solution would be to make findDependents take into account meta.roots. Something like:

// https://github.com/alexisvincent/systemjs-hmr/blob/master/lib/next.js#L133
if (!dependents.has(dep) && !roots.includes(dep)) {
    /* ... */
}

Where roots will be passed to it from System.reload. This way, a user of the library can define the points at which reloading stops.

srolel avatar Dec 19 '16 21:12 srolel

@Mosho1 The only issue with that would be that the lib has to traverse the tree to determine children it needs to delete. But something like that would be nice and general... I wonder if it's worth being this general. Can you think of a reason we might want to stop somewhere other then just the node or its roots?

alexisvincent avatar Dec 22 '16 11:12 alexisvincent

You can have all kinds of custom reloading scenarios, I don't see a reason to limit it to just the module itself or the roots.

Off the top of my head, say you have multiple data stores that aggregate to one big store (like redux stores) where you want the whole store to reload on change to any of its child nodes, but you might not want the whole app to render again. So any reloads to the child nodes should reach the aggregate store which has the actual reloading logic and then stop.

srolel avatar Dec 25 '16 09:12 srolel