Cannot use url.format with url.parse

[DanielHeath]

Using flow 0.16

I have no idea why this doesn't check (and why the error is so big); a parsed URL should be a valid parameter to url.format

Sample program:

/* @flow */

import url from "url";

export function roundTripUrl(urlStr: string): string {
    return url.format(url.parse(urlStr));
}

Output:

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:837:16,21: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:852:16,21: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:838:15,21: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:853:15,21: boolean

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:839:12,17: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:854:12,17: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:840:12,17: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:857:12,17: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:841:12,17: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:856:12,26: union type

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:842:16,21: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:855:16,21: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:843:12,17: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:861:12,17: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:844:14,19: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:859:14,19: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:845:13,15: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:860:13,18: object type

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:846:16,21: null
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:858:16,21: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:851:12,17: undefined
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:848:11,16: string

/Users/daniel.heath/Projects/ask-izzy/flowtest/foo.js:6:18,32: call of method parse
Error:
/private/tmp/flow/flowlib_1030271d/node.js:856:21,26: number
This type is incompatible with
/private/tmp/flow/flowlib_1030271d/node.js:841:12,17: string

Found 12 errors

[samwgoldman]

So, this one is a bit tricky, unfortunately. There are two separate issues here: one that we can fix, and one that requires a bit more work.

The issue that we can fix: the declared types of parse and format are a bit incompatible. For reference:

declare module "url" {
  declare function parse(
    urlStr: string,
    parseQueryString?: boolean,
    slashesDenoteHost?: boolean
  ): {
    protocol: ?string;
    slashes: ?boolean;
    auth: ?string;
    host: ?string;
    port: ?string;
    hostname: ?string;
    hash: ?string;
    search: ?string;
    query: ?any; // null | string | Object
    pathname: ?string;
    path: ?string;
    href: string;
  };
  declare function format(urlObj: {
    href?: string;
    protocol?: string;
    slashes?: boolean;
    auth?: string;
    hostname?: string;
    port?: string | number;
    host?: string;
    pathname?: string;
    search?: string;
    query?: Object;
    hash?: string;
  }): string;
  declare function resolve(from: string, to: string): string;
}

While parse returns an object with maybe values (null, undefined, or the value), format accepts an object with optional values (undefined or the value—no null). This is wrong. We want format to accept optional maybe values, e.g., { foo?: ?bar }. An updated lib def might look like this:

declare module "url" {
  declare function parse(
    urlStr: string,
    parseQueryString?: boolean,
    slashesDenoteHost?: boolean
  ): {
    auth: ?string;
    hash: ?string;
    host: ?string;
    hostname: ?string;
    href: string;
    path: ?string;
    pathname: ?string;
    port: ?string;
    protocol: ?string;
    query: ?(string | Object);
    search: ?string;
    slashes: ?boolean;
  };
  declare function format(urlObj: {
    auth?: ?string;
    hash?: ?string;
    host?: ?string;
    hostname?: ?string;
    href?: string;
    pathname?: ?string;
    port?: ?(string | number);
    protocol?: ?string;
    query?: ?(string | Object);
    search?: ?string;
    slashes?: ?boolean;
  }): string;
  declare function resolve(from: string, to: string): string;
}

However, we still get errors using this, although they are fewer:

url/url.js:4:12,32: call of method `parse`
Error:
[LIB] node.js:893:12,17: undefined
This type is incompatible with
[LIB] node.js:879:11,16: string

url/url.js:4:12,32: call of method `parse`
Error:
[LIB] node.js:895:23,28: number
This type is incompatible with
[LIB] node.js:882:12,17: string

These errors are more specific, and we can trace them down to the props which are different between parse's return type and format's param type: href and port.

parse returns href as string and format accepts href as void | string. This is valid by subtyping, but Flow is actually unifying these types, so we see an incompatibility error between void and string, above. Something similar is going on with port.

So this unification is tricky, because Flow is doing it for a good reason, which doesn't happen to apply in this case. Objects in JavaScript are mutable, so we can imagine many examples where this unification is necessary.

/* @flow */

function foo(): { foo: string } {
  return { foo: "foo" };
}

function bar(o: { foo?: string }): void {
  delete o.foo;
}

var o: { foo: string } = foo();
bar(o);
o.foo.length; // BOOM

What we need here is some way to declare that format does not mutate it's parameter, which would allow Flow to safely use subtyping instead of unification here. Ideally, this kind of annotation would also allow us to annotate the above function bar, such that the delete o.foo statement would error, because a modification to the "const" object type is disallowed.

I hope this explanation was clear. Flow really, really needs const values for this kind of thing. I'm not sure anyone is working on it at the moment, but it's a well known issue to the team.

Thanks for submitting this interesting issue!

[MoOx]

I am having a similar issue while trying to do the opposite: parse(format(aParsedUrlObjectThatHaveBeenMutated)).
It's not clear for my why the opposite cannot work since result of a parsed url seems stricter than what format accepts.

[MoOx]

Workaround (kind of weird to accept this)

parse(format({
    href: baseUrl.href,
    protocol: baseUrl.protocol,
    slashes: baseUrl.slashes,
    auth: baseUrl.auth,
    hostname: baseUrl.hostname,
    port: baseUrl.port,
    host: baseUrl.host,
    pathname: baseUrl.pathname,
    search: baseUrl.search,
    query: baseUrl.query,
    hash: baseUrl.hash,
  }))