8000 Correct HooksObject typing due to normalization; add finally by evankennedy · Pull Request #1300 · feathersjs/feathers · GitHub
[go: up one dir, main page]
Skip to content

Correct HooksObject typing due to normalization; add finally#1300

Merged
daffl merged 3 commits i 8000 ntofeathersjs:masterfrom
evankennedy:master
Apr 22, 2019
Merged

Correct HooksObject typing due to normalization; add finally#1300
daffl merged 3 commits intofeathersjs:masterfrom
evankennedy:master

Conversation

@evankennedy
Copy link
Contributor
@evankennedy evankennedy commented Apr 21, 2019

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:

      feathers/packages/commons/src/hooks.ts

      Lines 69 to 85 in 656bae7

      // 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:

      feathers/packages/feathers/lib/hooks/index.js

      Lines 156 to 159 in 3ad094e

      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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@evankennedy @daffl
0