lukeed / polka

A micro web server so fast, it'll make you dance! :dancers:
MIT License
5.39k stars 171 forks source link

TypeScript typings support? #17

Open motss opened 6 years ago

phra commented 6 years ago

can't we reuse the definition from @types/express? @lukeed do you have documented somewhere all the changes from the express apis? (if any :slightly_smiling_face: )

Youngestdev commented 6 years ago

@phra Full documentation will be available as version 1.0 is released.. ! Polka is still undergoing developments .. cheers :tada:

lukeed commented 6 years ago

I don't use Typescript, so this is a low priority for me, or could come from someone else! I imagine it'd be pretty simple since Polka is just a few prototypes mixed in with the native HTTP classes. If ClientRequest and ServerResponse are already typed, awesome! I would have thought that those two would be the trickiest parts.

orefalo commented 5 years ago

+1 also missing this

lukeed commented 5 years ago

Agreed – this would be nice to have. I still don't use TypeScript, but know that it's crucial for many others.

However, I don't like the idea of shipping typings down with every installation. And the DefinitelyTyped project has picked up a lot of momentum, which I'm glad to see as it addresses the same concerns I have.

So, sometime during (or after) this next push for a minor release, I'll make a PR to add Polka typings to the collection so that they're available as @types/polka 🤞 That will also keep the types in the vicinity of & in the hands of people who actually use TS and are good at this stuff 😆

I'll keep this issue open as a reminder~

lukeed commented 5 years ago

This depends (mostly) on Trouter definitions, which I plan to provide as well. Since both Polka & Trouter will have synchronized major releases, these will come together.

Related: https://github.com/lukeed/trouter/issues/7

demaggus83 commented 5 years ago

@lukeed: Do you want me to do the typings? I would do the typings for send and send-type too. It shouldn't take much time and I love Polka and Typescript - so I would like to contribute ;)

lukeed commented 5 years ago

@stahlstift That would be awesome! 🎉

Do you mind getting it started in the DefinitelyTyped repo? I would still prefer that they live there (see reasons above), but I could maybe live with them being inside Polka directly. I guess it depends how much uproar there would be. Although, everyone is already installing @types/express for Express apps anyway.

Also, no need to worry about send – that's being removed entirely. The send-type package is taking its place & its name.

demaggus83 commented 5 years ago

I will start with a fork and put the types in directly. We could then discuss that "where to place" problem ;)

Edit: After some research it would be more logical to push the types into @types. I need to start with Trouter because polka extends it. I am not experienced in writing d.ts files, so I will have a look into it and plan to be done, at least with Trouter, tomorrow.

lukeed commented 5 years ago

Of course 😄 Yes, the extends functionality they have inside the DefTyped repo is a great addition. I saw it for the first time when looking at Express' typings.

Starting with Trouter would be the best 1st step – you can look at this commit (https://github.com/lukeed/trouter/commit/abf5fa2242d066346e3449c719683f38379ea139) for a jump start. Adding on the Polka parts afterwards "should be easy" ™️ lol

demaggus83 commented 5 years ago

Status:

@lukeed It would be nice if you could ping me if there are API changes in one of this projects to keep the typings in sync

lukeed commented 5 years ago

@stahlstift that's great! Once I'm near a computer I'll give that a review. I've contributed to DT before so that should help in landing that PR.

As for Polka, the only breaking API change (that I can think of atm) is that handler now expects/receives a next<Function> as its third argument. A bunch of the internals changed, but I don't think that should affect the typings at all.

There's a new discord channel if you wanna swing by for quicker feedback 😆 https://discord.gg/xrRvfYV

revskill10 commented 5 years ago
declare module 'polka' {
  const content: any;
  export default content;
}
lukeed commented 5 years ago

Haha @frzi has completed a solid set of definitions for the 1.0. I've checked them out & they look great.

They'll be available as @types/polka with the release. At this stage, I'm wrapping up 1.0's "compat" layer for existing Express apps & plugging Polka into real-world production usage to see if anything pops up.

frzi commented 5 years ago

Yep. Waiting for stable 1.0 release before sending a PR to DefinitelyTyped. :)

Have been using the type definitions with some personal projects for a while now and liking the way things look! Can't wait!

Organizzzm commented 4 years ago

How long we should wait? I thought that it is straightforward task

lukeed commented 4 years ago

It is, thanks for clarifying 👍

MontoyaAndres commented 4 years ago

Hey, I'm currently using polka with Google cloud functions, Is there any types defined to use when the package @types/polka is ready? I don't know if this works

frzi commented 4 years ago

It's slightly outdated and a rather old contestant for @types. I'm really forward to working with polka again soon. When the time comes and someone else hasn't beaten me to the punch yet, I'll be sure to have the type definitions complete and ready! 💪

lukeed commented 4 years ago

I have been working more with TypeScript over the last ~6 months and have maintained definitions while working on/with polka@next for large production applications.

Here are the definitions I've been using, which stemmed from @frzi's original work (thank you): https://gist.github.com/lukeed/2fb80f9f87ee40204ef8412963348098

I've been using them like this:


import { IError, Middleware, Request, RequestHandler, Response, ErrorHandler, Polka } from 'polka';

// Extend `Request` specifically for this application
// ~> needed when doing stuff like: `req.user = await User.find(...)`
export interface IRequest extends Request {
    user?: My_User;
    items?: My_Item[];
    // ...
}

// Export wrapped types for <this> application
// ~> everything bound to the custom `IRequest` context
export type IRouter = Polka<IRequest>;
export type IMiddleware = Middleware<IRequest>;
export type IErrorHandler = ErrorHandler<IRequest>;
export type IHandler = RequestHandler<IRequest>;

// Custom error handler, if any
const onError: IErrorHandler = (err: IError, _req: IRequest, res: Response) => {
    const code = (err.code || err.status || 500);
    if (code >= 500) console.error('onError', err);
    res.headersSent || toError(res, code, err);
}

// Define custom router, with custom Request interface
export default function (): IRouter {
    return polka<IRequest>({ onError });
}
ConsoleTVs commented 4 years ago

Is it that hard to migrate the little lines of code polka claims to have to typescript? Honestly, if it have few lines of code, what's the issue? Not like you have to write a bunch of stuff more, and you make the library usable to typescript without having to maintaing a separate d.ts file.

'polka' is declared but its value is never read.ts(6133)
Could not find a declaration file for module 'polka'.
'**************************************' implicitly has an 'any' type.
  Try `npm install @types/polka` if it exists or add a new declaration (.d.ts) file containing `declare module 'polka';`ts(7016)
JamesMessinger commented 4 years ago

FYI - Here are type definitions that I created for Polka.

declare module "polka" {
  import { IncomingMessage, Server, ServerResponse } from "http";
  import { Url } from "url";
  import Trouter = require("trouter");

  namespace polka {
    /**
     * A middleware function
     */
    type Middleware = (req: IncomingMessage, res: ServerResponse, next: Next) => void | Promise<void>;

    /**
     * Calls the next middleware function in the chain, or throws an error.
     */
    type Next = (err?: string | Error) => void;

    /**
     * An `http.IncomingMessage`, extended by Polka
     */
    interface Request extends IncomingMessage {
      /**
       * The originally-requested URL, including parent router segments.
       */
      originalUrl: string;

      /**
       * The path portion of the requested URL.
       */
      path: string;

      /**
       * The values of named parameters within your route pattern
       */
      params: {
        [key: string]: string;
      };

      /**
       * The un-parsed querystring
       */
      search: string | null;

      /**
       * The parsed querystring
       */
      query: {
        [key: string]: string | string[];
      };
    }

    /**
     * An instance of the Polka router.
     */
    interface Polka extends Trouter<Middleware> {
      /**
       * Parses the `req.url` property of the given request.
       */
      parse(req: Request): Url;

      /**
       * Attach middleware(s) and/or sub-application(s) to the server.
       * These will execute before your routes' handlers.
       */
      use(...handlers: Middleware[]): this;

      /**
       * Attach middleware(s) and/or sub-application(s) to the server.
       * These will execute before your routes' handlers.
       */
      use(pattern: string | RegExp, ...handlers: Middleware[]): this;

      /**
       * Boots (or creates) the underlying `http.Server` for the first time.
       */
      listen(port?: number, hostname?: string): this;

      /**
       * Boots (or creates) the underlying `http.Server` for the first time.
       * All arguments are passed to server.listen directly with no changes.
       */
      listen(...args: unknown[]): this;

      /**
       * The main Polka `IncomingMessage` handler.
       * It receives all requests and tries to match the incoming URL against known routes.
       */
      handler(req: Request, res: ServerResponse, parsed?: Url): void;
    }

    /**
     * Polka options
     */
    interface Options {
      /**
       * The server instance to use when `polka.listen()` is called.
       */
      server?: Server;

      /**
       * A catch-all error handler; executed whenever a middleware throws an error.
       */
      onError?(err: Error, req: Request, res: ServerResponse, next: Next): void;

      /**
       * A handler when no route definitions were matched.
       */
      onNoMatch?(req: Request, res: ServerResponse): void;
    }
  }

  /**
   * Creates a Polka HTTP router.
   *
   * @see https://github.com/lukeed/polka
   */
  const polka: (opts?: polka.Options) => polka.Polka;

  export = polka;
}
lukeed commented 4 years ago

Thank you @JamesMessinger - I believe those are for current Polka (stable), right?

JamesMessinger commented 4 years ago

Yeah, that's for 0.5.2. I hadn't actually realized that you had a 1.0.0 in the works 😅 When is that expected to be released?

JamesMessinger commented 4 years ago

What things have changed in 1.0.0? I'll be happy to update my type definition.

lukeed commented 4 years ago

Cool, thanks :) I believed that to be the case, otherwise there were some definite issues with that for the current polka@next version.

I posted the WIP 1.0 types here: https://github.com/lukeed/polka/issues/17#issuecomment-545119126. I've been using them for months now quite successfully. There might be a slight modification or two needed that I've included in my projects, but I'll have to review notes.

Lastly, I left my job – I'll finally be able to put in the time that a few of my OSS projects have needed. Polka is high on the to-be-finished list during this time.

JamesMessinger commented 4 years ago

Oh, cool! Thanks for linking to your own type definitions. I hadn't noticed those before.

Looking forward to seeing Polka 1.0.0 soon. I've been searching for an Express replacement that's as minimalist/performant as possible while still compatible with most middleware packages. Polka fits the bill perfectly. Thank you for creating it and keeping it lightweight!

pkuczynski commented 4 years ago

I created a PR for DefinietlyTyped to cover current stable version: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44943

khc commented 4 years ago

There is a bug in this types and prevents chaining .listen() to .get() and other Trouter methods.

Property 'listen' does not exist on type 'Trouter<T>'.
pkuczynski commented 4 years ago

@khc could you provide here a usage example based on https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/polka/polka-tests.ts

I will then fire up a new PR with fixes.

I am not heavy polka user and I simply followed what @JamesMessinger did, adopting it to DefnietlyTyped repo.

benmccann commented 4 years ago

I wasn't able to use the DefinitelyTyped package with middlewares.

In package.json: "@types/compression": "^1.7.0",

In server.ts:

import compression from 'compression';

const { PORT, NODE_ENV } = process.env;
const dev = NODE_ENV === 'development';

polka()
    .use(
        compression({ threshold: 0 })
    )

Gives error:

> @rollup/plugin-typescript TS2769: No overload matches this call.
  Overload 1 of 2, '(...handlers: Middleware[]): Polka', gave the following error.
    Argument of type 'RequestHandler<ParamsDictionary, any, any, ParsedQs>' is not assignable to parameter of type 'Middleware'.
      Types of parameters 'req' and 'req' are incompatible.
        Type 'IncomingMessage' is missing the following properties from type 'Request<ParamsDictionary, any, any, ParsedQs>': get, header, accepts, acceptsCharsets, and 26 more.
  Overload 2 of 2, '(pattern: string | RegExp, ...handlers: Middleware[] | Polka[]): Polka', gave the following error.
    Argument of type 'RequestHandler<ParamsDictionary, any, any, ParsedQs>' is not assignable to parameter of type 'string | RegExp'.
      Type 'RequestHandler<ParamsDictionary, any, any, ParsedQs>' is missing the following properties from type 'RegExp': exec, test, source, global, and 12 more.
khc commented 4 years ago

@pkuczynski Your tests are missing .listen() after Trouter methods. I think the idea is, that even though Polka extends Trouter, it's methods returns Polka instance, not Trouter itself.

Your types for Polka also extends Trouter, but you covered only Polka methods, leaving original Trouter methods, which by Trouter types returns Trouter instance.

For example, this will work:

const app = polka(); app.get('/', hello()); app.listen(3000);

And this will not

polka().get('/', hello()).listen(3000);

I hope I explained it clear enough :)

ajbouh commented 4 years ago

The current polka types have the following:

    /**
     * Parses the `req.url` property of the given request.
     */
    parse(req: Request): Url;

and

    /**
     * The main Polka `IncomingMessage` handler.
     * It receives all requests and tries to match the incoming URL against known routes.
     */
    handler(req: Request, res: ServerResponse, parsed?: Url): void;

Based on my reading of the code and the documentation, req should be an IncomingMessage and not a Request.

Does this sound right to others?

pkuczynski commented 4 years ago

Based on my reading of the code and the documentation, req should be an IncomingMessage and not a Request.

Polka decorates IncomingMessage with some extra things described as Request interface. How it is done now, is correct.

pkuczynski commented 4 years ago

There is a bug in this types and prevents chaining .listen() to .get() and other Trouter methods.

I just checked out and it works for chained... Will add some more tests in DT to prove it...

pkuczynski commented 4 years ago

I wasn't able to use the DefinitelyTyped package with middlewares.

I created a PR fixing this issue: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/45060

The only thing I am not sure if Polka should rely on express types not havibg it in its own dependencies? :/

JoCat commented 1 month ago

The only thing I am not sure if Polka should rely on express types not havibg it in its own dependencies? :/

Definitely not, otherwise you might run into a point where a function that doesn't exist in Polka will be returned in types. As for example here: image image