feat(core): Improve legacy hooks integration

[vonagam]

Main thing is that now legacy hooks wrappers are cached much more. Currently collectLegacyHooks creates new array every time it is called with afterHooks being spread/reversed, errorHook being produced again and then all legacy combined into one array. Those things can be done during .hooks setup call.

Also:

• Added fromBeforeHooks, fromAfterHooks and fromErrorHook to complete the set.
• Typed better those helpers.
• Removed types argument from enableLegacyHooks, it lacked a meaning.
• Added all to list of protectedMethods for services.

[vonagam]

Had to break down enableLegacyHooks into pieces because of high Cognitive Complexity. But then one part, that it was split into, was still not passing the check... Then i found out that if you use _.each() or array.forEach() it does not directly increase the score (only nesting), while for-of loop does, which penalizes it. for-of is better than alternatives (performance, integration with async/generators and no dependencies), so it is strange that its usage ends up being punished... Last commit about that and should not be merged.

Real problem here though is that the passing level is set too low (maybe because of replacements for for-of that are used in code and lower the score). Here the author of the indicator says that 15 is a recommended maximum. If you want lower, maybe some middle ground will do - between 11 and 13.

[daffl]

This looks great, thank you!

I've been looking with @marshallswain into a way to make the transition less scary (early adopters with larger codebases had concerns about the old hooks becoming "legacy") and we were thinking of using a naming of "Regular" (before, after, error) and "Async" hooks which sparked the idea of adding the regular hooks into @feathersjs/hooks. The PR for that is at feathersjs/hooks#92 which basically allows you to do something like:

import { hooks } from '@feathersjs/hooks';

class DummyClass {
    async create(data: any) {
        return data;
    }
}

hooks(DummyClass, {
    create: middleware([
        collect({
            before: [],
            after: [],
            error: []
        })
    ])
})

So I'm thinking of getting your PR in, porting it into the @feathersjs/hook PR and then integrating the new version here.

[vonagam]

Sounds good.

Small note - it might make sense to add finally as an allowed hook type. It was supported before unofficially and is trivial to add - it is like before hook but uses Promise.finally instead of Promise.then and should go first in order.

[marshallswain]

I've copied the utility functions into the hooks repo in the deno-port branch. I simplified the typings since @feathersjs/hooks doesn't know about the Application and Service context.

https://github.com/feathersjs/hooks/blob/deno-port/packages/hooks/src/regular.ts