thgh / rollup-plugin-serve

Serve your rolled up bundle like webpack-dev-server
MIT License
248 stars 54 forks source link

Improvements to Type Definitions #92

Closed tommy-mitchell closed 1 year ago

tommy-mitchell commented 2 years ago

Originally I came here to file a bug report. In index.d.ts, the options.headers property is declared using TypeScript's builtin Headers interface, which is different from the intended Node HTTP headers:

https://github.com/thgh/rollup-plugin-serve/blob/6743a314411d24c373329d61236e215b7af20b0a/index.d.ts#L53-L56

However, while looking into it, I noticed that many of the type descriptions were slightly different from those declared in index.js using JSDoc tags. The TypeScript compiler actually already has a way of emitting type definitions from JS files, which can use the plugin's already-maintained JSDoc tags (and their comment descriptions) to create an index.d.ts file. Using this method, types could be automatically created on build and would only have to be maintained in the one file.

I've tested this out on a local clone of the repo and it seems to work well. I'm happy to make a PR.


As an aside, the DefinitelyTyped definitions for Node HTTP includes a list of common headers, such as this list for IncomingHTTPHeaders. I'm not familiar with the types of headers used by this plugin, but by using these types IntelliSense can suggest the common headers[^1]:

import { IncomingHttpHeaders, OutgoingHttpHeaders } from 'http'

export interface ServeOptions {
  // ...

  headers?: IncomingHttpHeaders | OutgoingHttpHeaders | {
    // i.e. Parameters<OutgoingMessage["setHeader"]>
    [name: string]: number | string | ReadonlyArray<string>
  }
}

image

[^1]: Using TypeScript here as I'm more familiar with the syntax than with JSDoc. Idea still applies.

thgh commented 2 years ago

Makes sense! Do you mind reviewing the PR?

richex-cn commented 1 year ago

Just found this. Just what I need~

Snipaste_2022-11-23_17-34-22

thgh commented 1 year ago

Lost track of this. I will merge the PR.

@richex-cn consider lowercasing the header

richex-cn commented 1 year ago

consider lowercasing the header

It still not work for me.

image

And #93 will solve it.

image