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

stringifyUrl type should accept string[] to be consistent with parse #278

Closed
esetnik opened this issue Sep 28, 2020 · 2 comments · Fixed by #279
Closed

stringifyUrl type should accept string[] to be consistent with parse #278

esetnik opened this issue Sep 28, 2020 · 2 comments · Fixed by #279

Comments

@esetnik
Copy link
Contributor

esetnik commented Sep 28, 2020

Example

  const queryStringParams = queryString.parse(search);
  const sourceInjectedQueryString = queryString.stringifyUrl({
    url: source,
    query: queryStringParams,
  });

Error

[tsserver 2322] [E] Type 'ParsedQuery<string>' is not assignable to type 'Record<string, string | null | undefined>'.
  Index signatures are incompatible.
    Type 'string | string[] | null' is not assignable to type 'string | null | undefined'.
      Type 'string[]' is not assignable to type 'string'.

This is a regression introduced by b15f945

ParsedQuery type has array

export interface ParsedQuery<T = string> {
	[key: string]: T | T[] | null;
}

UrlObject type does not support an array

export interface UrlObject {
	readonly url: string;

	/**
	Qverrides queries in the `url` property.
	*/
	readonly query: Record<string, string | undefined | null>;

	/**
	Overrides the fragment identifier in the `url` property.
	*/
	readonly fragmentIdentifier?: string;
}

Note

Additionally it should support number and boolean since these are also stringifiable.

The type of object argument in stringify should also match the type of query in stringifyUrl as they both stringify a record.

Proposed Fix

export type Stringifiable = string | boolean | number;
export type StringifiableRecord = Record<
  string,
  Stringifiable | Stringifiable[] | null | undefined
>;

export interface UrlObject {
  readonly url: string;

  /**
	Qverrides queries in the `url` property.
	*/
  readonly query: StringifiableRecord;

  /**
	Overrides the fragment identifier in the `url` property.
	*/
  readonly fragmentIdentifier?: string;
}

export function stringify(
  object: StringifiableRecord,
  options?: StringifyOptions
): string;
@sindresorhus
Copy link
Owner

Your proposed fix looks good. Would you be interested in doing a pull request with it?

@esetnik
Copy link
Contributor Author

esetnik commented Sep 28, 2020

Your proposed fix looks good. Would you be interested in doing a pull request with it?

Happy to do that. Are you good with the terminology I used?

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

Successfully merging a pull request may close this issue.

2 participants