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

The return type for the request handler should be a response handler and not void. #285

Open
LuisAverhoff opened this issue Dec 10, 2019 · 2 comments

Comments

@LuisAverhoff
Copy link

LuisAverhoff commented Dec 10, 2019

There is a type error that occurs if you try to get the number of calls a handler has made. In the index.d.ts file, the return type for any request http verb is void, when in the actual source code it is a function that returns a handler; particularly, the handler that was passed to the register function. This handler is the one that maintains state of how many calls has been made. Here is an example of how it is being used in the READMe.md file.

server.get(/* ... */);
const handler = server.handlers[0]; // type error. handlers does not exist as a type for the server.

// or

const handler = server.get(/* ... */); // type error. http verb function returns void.

// then

const numberOfCalls = handler.numberOfCalls;

My suggestion is to change

export type RequestHandler = (
  urlExpression: string,
  response: ResponseHandler,
  asyncOrDelay?: boolean | number
) => void;

to

export type RequestHandler = (
  urlExpression: string,
  response: ResponseHandler,
  asyncOrDelay?: boolean | number
) => ResponseHandler;

For reference, here is what the verbify function is doing.

function verbify(verb) {
  return function(path, handler, async) {
    return this.register(verb, path, handler, async);
  };
}

and here is what the register function is doing.

 register: function register(verb, url, handler, async) {
    if (!handler) {
      throw new Error('The function you tried passing to Pretender to handle ' +
        verb + ' ' + url + ' is undefined or missing.');
    }

    handler.numberOfCalls = 0;
    handler.async = async;
    this.handlers.push(handler);

    var registry = this.hosts.forURL(url)[verb];

    registry.add([{
      path: parseURL(url).fullpath,
      handler: handler
    }]);

    return handler; // notice how we are returning a handler in the register function.
  },

Of course, that leaves the issue of the handler not having the properties numberOfCalls and async as those seem to get added dynamically.

So the interface for ResponseHandler will also need to get updated but I dont know much about the codebase to get this done smoothly. Hopefully it should not be to much work to add these missing properties to the handler.

@LuisAverhoff LuisAverhoff changed the title The type for the request handler should be a handler and not void. The return type for the request handler should be a response handler and not void. Dec 10, 2019
@xg-wang
Copy link
Member

xg-wang commented Dec 22, 2019

@mike-north any idea how to address the dynamically added properties type?

@mike-north
Copy link
Member

@xg-wang the easiest way to handle this would be something like

 register: function register(verb, url, handler: RequestHandler, async: Async) {
    // Locally, work with a value w/ optional properties
    const h: RequestHandler & { numberOfCalls?: number, async?: Async } = handler;
    h.numberOfCalls = 0;
    h.async = async;
    this.handlers.push(h);

    var registry = this.hosts.forURL(url)[verb];

    registry.add([{
      path: parseURL(url).fullpath,
      handler: h
    }]);
   // Ultimately return a type with non-optional extra properties
    return h as RequestHandler & { numberOfCalls: number; async: Async }
  },

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

No branches or pull requests

3 participants