custom-module: Call reload() function on auto-reload

[matthijskooijman]

This allows modules to re-initialize any state they need whenever the module is reloaded.

Without this, any such initialization can also be done in the top level of the custom module, but this has two problems:

1. Top-level module code runs on load and reload, and for the initial load, it runs before the connections are set up, so trying to use send() would produce an error on load (not reload).

2. Top-level module code runs for the primary custom module, but also for any nested modules, removing control about the process and ordering from the primary custom module. In addition, a module that can be used both as primary and nested module, there is no clean way to distinguish between these cases.

Both of these problems can be solved by doing some bookkeeping in the global object, but that produces messy code.

This commit allows modules to define a reload() function, which is called (for the primary module only) after the module is reloaded. This means that any initialization for the first load can be done in init() and for reloads in reload(), and in both cases all connections will already have been set up solving issue 1. In addition, neither of these functions are called for nested modules, solving issue 2 (and the primary module can then stil explicitly call a reload function on nested modules if needed, as shown in the examples).

[jean-emmanuel]

Thanks for the thorough explanation, the PR looks fine to me (as does #852) ! I'll merge them within a few days, I'll just take some time to think about it before I do.

I'm realizing now that module.exports.unload() is called for all submodules in spite of what the example suggests, I think it ought to behave that way but it might appear a little confusing/inconsistent with the addition of reload() as proposed. Nothing critical really, just food for thought.

[matthijskooijman]

I'll merge them within a few days, I'll just take some time to think about it before I do.

Cool. Leaving some time to turn things over in your head is always a good idea ;-)

I'm realizing now that module.exports.unload() is called for all submodules in spite of what the example suggests, I think it ought to behave that way but it might appear a little confusing/inconsistent with the addition of reload() as proposed. Nothing critical really, just food for thought.

Good point, I had not realized unload works like that. But indeed even though it might be a bit surprising, I do think it makes sense, since unload() should handle cleanup and thus it is better if the main module is not involved and cleanup always just works. For init() and reload() you could also choose to run that on all submodules, except that it makes sense to let the main module keep control over the initialization process. This also allows my suggested usecase of using a module as main or nested module. And if modules are loaded lazily (i.e. in response to events), their init() and reload() cannot even be called by open-stage-control (but that is just a minor point).

I'll update the example and docs to reflect how it works. If you think it should work differently after all, let's discuss that :-)

[matthijskooijman]

I've updated the example and docs, I added this commit: https://github.com/jean-emmanuel/open-stage-control/pull/851/commits/a3194a3bb8bde765774359e1f4608c395e787216 and I made these changes to the second commit (I already squashed that fixup commit into the second commit you already saw, just linking this commit for easier review. Note that the fixup commit message is actually wrong (references the wrong commit), but I squased it correctly): https://github.com/jean-emmanuel/open-stage-control/commit/c187cb0a1d16fbcc8a2559512f97dc621e3e0080

[jean-emmanuel]

Merged, thank you !!