Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Correct HooksObject typing due to normalization; add finally #1300

Merged
merged 3 commits into from
Apr 22, 2019

Conversation

evankennedy
Copy link
Contributor

Summary

  • Tell us about the problem your pull request is solving.
    This pull request does 2 things:
    • allows for the generalized format of hook objects not requiring a HookMap (aka all, find, etc)
      The standardization of the object occurs here:
      // Converts different hook registration formats into the
      // same internal format
      export function convertHookData (obj: any) {
      let hook: any = {};
      if (Array.isArray(obj)) {
      hook = { all: obj };
      } else if (typeof obj !== 'object') {
      hook = { all: [ obj ] };
      } else {
      each(obj, function (value, key) {
      hook[key] = !Array.isArray(value) ? [ value ] : value;
      });
      }
      return hook;
      }
    • adds the hook finally method to typings
      The code enabling this is in a few places throughout the file, but can be seen here:
      before: getHooks(app, service, 'before', method),
      after: getHooks(app, service, 'after', method, true),
      error: getHooks(app, service, 'error', method, true),
      finally: getHooks(app, service, 'finally', method, true)
  • Are there any open issues that are related to this?
    No, I didn't see any
  • Is this PR dependent on PRs in other repos?
    No

Other Information

I noticed finally is not listed in the docs: https://docs.feathersjs.com/api/hooks.html#registering-hooks
If it is not encouraged and will be removed in a future release, I suggest we add @deprecated, otherwise I believe it should be added as-is.

I added a PR for "DefinitelyTyped" here DefinitelyTyped/DefinitelyTyped#34897

@daffl daffl merged commit b28058c into feathersjs:master Apr 22, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants