Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

docs: d.ts of resources& logger #2079

Merged
merged 3 commits into from
Feb 3, 2018
Merged

docs: d.ts of resources& logger #2079

merged 3 commits into from
Feb 3, 2018

Conversation

x22x22
Copy link
Contributor

@x22x22 x22x22 commented Feb 2, 2018

  1. 补充resources中的fn类型可能为Controller
  2. 补充Logger中的args的类型可能为number或object
Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

1. 补充resources中的fn类型可能为Controller
1. 补充Logger中的args的类型可能为number或object
index.d.ts Outdated
@@ -522,7 +524,7 @@ export interface Application extends EggApplication {
/**
* restful router api
*/
resources(name: string, prefix: string, fn: string): Router;
resources(name: string, prefix: string, fn: string | Controller): Router;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这最后接收的参数不是 Controller,参考前面 get、post、put 那些方法的声明

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resources(path: string, prefix: string, ...middleware: any[]): Router;
改成这样可否?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/**
 * 1. split (name, url, ...middleware, controller) to
 * {
 *   prefix: [name, url]
 *   middlewares [...middleware, controller]
 * }
 *
 * 2. resolve controller from string to function
 *
 * @param  {Object} options inputs
 * @param {Object} options.args router params
 * @param {Object} options.app egg application instance
 * @return {Object} prefix and middlewares
 */
function spliteAndResolveRouterParams({ args, app }) {
  let prefix;
  let middlewares;
  if (args.length >= 3 && (is.string(args[1]) || is.regExp(args[1]))) {
    // app.get(name, url, [...middleware], controller)
    prefix = args.slice(0, 2);
    middlewares = args.slice(2);
  } else {
    // app.get(url, [...middleware], controller)
    prefix = args.slice(0, 1);
    middlewares = args.slice(1);
  }
  // resolve controller
  const controller = middlewares.pop();
  middlewares.push(resolveController(controller, app));
  return { prefix, middlewares };
}
middlewares = args.slice(2);

那么应该可以是

resources(path: string, prefix: string, ...middleware: any[]): Router;

或者

resources(path: string, prefix: string, middleware: any): Router;

@codecov-io
Copy link

codecov-io commented Feb 2, 2018

Codecov Report

Merging #2079 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2079   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files          29       29           
  Lines         743      743           
=======================================
  Hits          740      740           
  Misses          3        3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbfacc5...368a94c. Read the comment docs.

index.d.ts Outdated
warn(info: string, ...args: string[]): void;
debug(info: string, ...args: string[]): void;
error(info: string | Error, ...args: string[]): void;
info(info: string, ...args: LogArgs[]): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个感觉用 any 更合适,这个会调用 util.format 做处理,而 node 的官方申明文件中 util.format 的入参就是 any,不限类型

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以, 我马上改. 只是怕你们不接受any而已...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

该是什么就什么 -.- 没啥不接受的,这里本来就是可以接受任意参数的,而且没法穷举完,那肯定是 any。

顺便 rebase 下

DefinitelyTyped(resources&Logger): 补充类型描述
@atian25 atian25 changed the title DefinitelyTyped(resources&Logger): 补充类型描述 docs: d.ts of resources& logger Feb 2, 2018
@dead-horse dead-horse merged commit 03a8944 into eggjs:master Feb 3, 2018
popomore pushed a commit that referenced this pull request Feb 3, 2018
docs: d.ts of resources& logger (#2079)
@x22x22 x22x22 deleted the x22x22 branch February 3, 2018 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants