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

fix: Small improvements #2478

Merged
merged 11 commits into from
Nov 7, 2021
Merged

fix: Small improvements #2478

merged 11 commits into from
Nov 7, 2021

Conversation

vonagam
Copy link
Member

@vonagam vonagam commented Oct 24, 2021

  • Added missing dependencies.
  • Renamed Application type parameters ServiceTypes to Services and AppSettings to Settings. That's the change that does not solve anything and a breakish one (since when somebody augment a module they have to use same names). Can remove.
  • Default T to any in Service type (and its relatives), so that you can simply write Service instead of Service<any>. There is some slight difference between Service/Service<any> and Service<any, any> - first one has D as Partial<any>. If this change comes up somewhere, there is a way to add new type later to default D to any if T is any.
  • authentication package had a problem with lib/index.d.ts where there was an import of @feathersjs/hooks which is not a direct dependency of the package, fixed by tweaking exports.
  • in transport-commons fix types for Publisher (wrong type for context arg) and Application.channel (not supporting an argument of type string | string[]).
  • Rename CustomMethod type to CustomMethods and allow to easily specify data and return type or keep it any as it currently is.
  • Pass hook enabled methods to enableLegacyHooks (so include custom and filter out unused ones).
  • In koa add headers to params (not sure if should be cloned as query is, in express neither query nor headers are cloned).
  • In koa add types for feathers props on koa context - feathers and hook.
  • In transport-commons and express remove hooks dependency and use correct type for context - it isn't supposed to work with generic hook context, only with feathers one.

@vonagam vonagam force-pushed the small-things branch 8 times, most recently from cb0862b to cfaa447 Compare October 26, 2021 13:58
@daffl
Copy link
Member

daffl commented Nov 7, 2021

These are great, thank you! I think the CustomMethods change makes sense, we'll just have to update the docs. I wonder if there is a better way in TypeScript to do that. We're having a similar problem making service method typings more flexible.

@daffl daffl merged commit b8eb804 into feathersjs:dove Nov 7, 2021
# 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