Add overloads to support passing single string arg to renderStatic

[jeremymcintyre]

The renderStatic method from PageForServer supports passing 1-2 arguments. This proposes adding some overloads to reflect the case where a single string argument is passed.

Due to the type ambiguity introduced for the first argument, also adds an additional typeof check. Depending on preference, an else clause could also be added to the first conditional instead.

Examples:

// string only
page.renderStatic('signup');

// number, string
page.renderStatic(401, 'login');

[craigbeck]

Looks goos to me. Do you have a code example of the additional overloads that you could add to the PR for documentation purposes?

[jeremymcintyre]

@craigbeck to be honest I'm not sure if it makes sense for this 3rd overload to have an optional first arg. The only reason I marked it as optional is because the original signature showed both args as optional.

If we should always at least pass one arg, then I would update to something like:

  renderStatic(ns: string): void;
  renderStatic(status: number, ns?: string): void;
  renderStatic(arg1: number | string, ns?: string) {

[ericyhwang]

In derby@3, we changed PageForServer#render, normalize the order of params to be render(ns: string, status?: number):
https://github.com/derbyjs/derby/releases/tag/v3.0.0

Longer term, we should aim to keep renderStatic consistent with that.

Unfortunately, we can only remove the renderStatic(status: number, ns: string) signature in a major version, so for now we'd need to support both signatures, with runtime type detection.

Perhaps something like this?

  renderStatic(ns: string, status?: number): void;
  renderStatic(status: number, ns: string): void;
  renderStatic(arg0: string | number, arg1?: number | string) {
    const ns = typeof arg0 === 'string' ? arg0 : arg1;
    const status = typeof arg1 === number ? arg1 : arg0;
    if (!ns) throw new Error('renderStatic ns param required');

[jeremymcintyre]

Thanks @ericyhwang - how would you recommend narrowing the type of ns to string? (I think it will still be typed as string | number after the existence check, and must be a string for the assignment of pageHtml and tailHtml below)

Maybe throw an additional error if ns is not a string? Or change the existence check to a type check?

  renderStatic(ns: string, status?: number): void;
  renderStatic(status: number, ns: string): void;
  renderStatic(arg0: string | number, arg1?: number | string) {
    const ns = typeof arg0 === 'string' ? arg0 : arg1;
    const status = typeof arg1 === 'number' ? arg1 : arg0;
    if (typeof ns !== 'string') throw new Error('renderStatic ns param required');

[jeremymcintyre]

Also is it okay that in the new overloads there is no overload where both arguments are marked optional, as it is before these changes?