fix(core): Allow to return a new hook context in basic hooks

[daffl]

This fixes a regression in backwards compatibility that is difficult to debug.

Closes #2451

[vonagam]

Wait, but it wasn't an unexpected regression, it was a deliberate choice for depreciation in next major version advocated by me in #1443.

This current fix, while small and easy creates a different behavior from v4. Here you just Object.assign returned object, before discarding it, to original context. Examples of problems that may lead to:

• If somebody to delete property (custom or error, id) from a new context, that change won't be transferred by Object.assign and will be lost.
• Map/WeakMap with context as a key will not work, since while one will think that they have a reference to new current context, what they actually have is an update to original one.
• As before, if you start making changes to new context object but then something throws an error, all your changes are lost, which creates difference in behavior with non-returning hooks.
• There is also a composition problem in v4 that i did not mention before - if you work with arbitrary passed-in hooks then you yourself should handle cases when they return contexts and the only consistent working way is to start returning too, which leads to propagation of a bad practice.

And migration is easy, they could have done the same that was done here - instead of return {...context, ...update} just write Object.assign(context, update) - it works (and more proper) both in v4 and v5.

Anyway, you can ignore this message, just had to write since it was me who proposed depreciation.

[daffl]

Ah yes, sorry I remember now. The problem with this approach was that I heard from a couple of beta testers now that they are ensuring immutability in their projects and did something like:

context => {
  return {
    ...context,
    params
  }
}

The problem with the new implementation was that it just did nothing and you had no idea why. You may be right in that it makes more sense to throw an error if a different context than the original one is returned.

[vonagam]

had no idea why

Legacy hook return type should have been updated to reflect that returning is not an option anymore. Then they could see (at least those who use typescript) an error at writing time instead of compile/runtime one.

ensuring immutability

Good intention, but stems from miscommunication (docs and types). Originally returning new context was added (as you said) with an idea of a pure hook (receive input, return output without side effects), but it was never actually implemented like that in the end. So no profits, only costs for that mirage...

Another smallish thing is that migration from void hooks to async hooks are easier than from returning ones. So if you plan to force migration to new hooks in the distant future (one or two versions down the line), they would still need to update.