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

TypeScript typings support? #17

Open
motss opened this issue Jan 24, 2018 · 36 comments
Open

TypeScript typings support? #17

motss opened this issue Jan 24, 2018 · 36 comments
Labels
Milestone

Comments

@motss
Copy link

motss commented Jan 24, 2018

No description provided.

@phra
Copy link

phra commented Jan 25, 2018

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

@Youngestdev
Copy link

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

@lukeed
Copy link
Owner

lukeed commented Jan 25, 2018

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
Copy link

orefalo commented Sep 16, 2018

+1 also missing this

@lukeed
Copy link
Owner

lukeed commented Sep 17, 2018

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
Copy link
Owner

lukeed commented Jan 2, 2019

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: lukeed/trouter#7

@lukeed lukeed added this to the 1.0.0 milestone Jan 2, 2019
@demaggus83
Copy link

demaggus83 commented Mar 2, 2019

@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
Copy link
Owner

lukeed commented Mar 2, 2019

@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
Copy link

demaggus83 commented Mar 2, 2019

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
Copy link
Owner

lukeed commented Mar 2, 2019

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 (lukeed/trouter@abf5fa2) for a jump start. Adding on the Polka parts afterwards "should be easy" ™️ lol

@demaggus83
Copy link

demaggus83 commented Mar 3, 2019

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
Copy link
Owner

lukeed commented Mar 3, 2019

@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
Copy link

declare module 'polka' {
  const content: any;
  export default content;
}

@lukeed
Copy link
Owner

lukeed commented Apr 8, 2019

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.

@lukeed lukeed added the has fix label Apr 8, 2019
@frzi
Copy link

frzi commented Apr 8, 2019

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
Copy link

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

@lukeed
Copy link
Owner

lukeed commented Sep 23, 2019

It is, thanks for clarifying 👍

@MontoyaAndres
Copy link

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
Copy link

frzi commented Oct 22, 2019

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
Copy link
Owner

lukeed commented Oct 22, 2019

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

This comment has been minimized.

@JamesMessinger
Copy link

JamesMessinger commented Mar 22, 2020

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
Copy link
Owner

lukeed commented Mar 22, 2020

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

@JamesMessinger
Copy link

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
Copy link

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

@lukeed
Copy link
Owner

lukeed commented Mar 22, 2020

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: #17 (comment). 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
Copy link

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
Copy link

I created a PR for DefinietlyTyped to cover current stable version: DefinitelyTyped/DefinitelyTyped#44943

@khc
Copy link

khc commented May 22, 2020

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
Copy link

@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
Copy link

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
Copy link

khc commented May 23, 2020

@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
Copy link

ajbouh commented May 25, 2020

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
Copy link

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
Copy link

pkuczynski commented May 26, 2020

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
Copy link

pkuczynski commented May 26, 2020

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

I created a PR fixing this issue: DefinitelyTyped/DefinitelyTyped#45060

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests