Strip undefined values from URL params; disallow some forms for params

Being able to specify `params` as a single record object is very
convenient, but it becomes inconvenient when some parameters are
optional. Previously you could not use the record object syntax if a
parameter is optional. Now you can use that syntax and we will strip
undefined values, like `JSON.stringify`.

Previously we allowed all of the various forms of data that can be
passed to `new URLSearchParams()` (and in fact just passed what we got
directly to there). Implementing this undefined-stripping for all of the
forms (like "arbitrary iterable object of two-element arrays") would be
a lot of code copied from the DOM implementation, but *not* stripping
undefined from the other forms would be inconsistent. So we change to
*only* allow the record form (which is the only form that our docs show)
plus providing `URLSearchParams` yourself; if you used one of the other
forms you can just add a `new URLSearchParams()` to your code.

See the changeset entry for more details.

Fixes #24.

.changeset/great-poems-share.md

@@ -0,0 +1,15 @@
+---
+'@apollo/datasource-rest': major
+---
+
+When passing `params` as an object, parameters with `undefined` values are now skipped, like with `JSON.stringify`. So you can write:
+
+```ts
+getPost(query: string | undefined) {
+ return this.get('post', { params: { query } });
+}
+```
+
+and if `query` is not provided, the `query` parameter will be left off of the URL instead of given the value `undefined`.
+
+As part of this change, we've removed the ability to provide `params` in formats other than this kind of object or as an `URLSearchParams` object. Previously, we allowed every form of input that could be passed to `new URLSearchParams()`. If you were using one of the other forms (like a pre-serialized URL string or an array of two-element arrays), just pass it directly to `new URLSearchParams`; note that the feature of stripping `undefined` values will not occur in this case. For example, you can replace `this.get('post', { params: [['query', query]] })` with `this.get('post', { params: new URLSearchParams([['query', query]]) })`. (`URLSearchParams` is available in Node as a global.)

src/RESTDataSource.ts

@@ -10,11 +10,16 @@ import type { WithRequired } from '@apollo/utils.withrequired';
 
 type ValueOrPromise<T> = T | Promise<T>;
 
-// URLSearchParams is globally available in Node / coming from @types/node
-type URLSearchParamsInit = ConstructorParameters<typeof URLSearchParams>[0];
-
 export type RequestOptions = FetcherRequestInit & {
- params?: URLSearchParamsInit;
+ /**
+ * URL search parameters can be provided either as a record object (in which
+ * case keys with `undefined` values are ignored) or as an URLSearchParams
+ * object. If you want to specify a parameter multiple times, use
+ * URLSearchParams with its "array of two-element arrays" constructor form.
+ * (The URLSearchParams object is globally available in Node, and provided to
+ * TypeScript by @types/node.)
+ */
+ params?: Record<string, string | undefined> | URLSearchParams;
  cacheOptions?:
  | CacheOptions
  | ((
@@ -195,6 +200,20 @@ export abstract class RESTDataSource {
  return this.fetch<TResult>(path, { method: 'DELETE', ...request });
  }
 
+ private urlSearchParamsFromRecord(
+ params: Record<string, string | undefined> | undefined,
+ ): URLSearchParams {
+ const usp = new URLSearchParams();
+ if (params) {
+ for (const [name, value] of Object.entries(params)) {
+ if (value !== undefined && value !== null) {
+ usp.set(name, value);
+ }
+ }
+ }
+ return usp;
+ }
+
  private async fetch<TResult>(
  path: string,
  request: DataSourceRequest,
@@ -205,7 +224,7 @@ export abstract class RESTDataSource {
  params:
  request.params instanceof URLSearchParams
  ? request.params
- : new URLSearchParams(request.params),
+ : this.urlSearchParamsFromRecord(request.params),
  headers: request.headers ?? Object.create(null),
  body: undefined,
  };
@@ -216,7 +235,7 @@ export abstract class RESTDataSource {
 
  const url = await this.resolveURL(path, modifiedRequest);
 
- // Append params to existing params in the path
+ // Append params from the request to any existing params in the path
  for (const [name, value] of modifiedRequest.params as URLSearchParams) {
  url.searchParams.append(name, value);
  }

src/__tests__/RESTDataSource.test.ts

@@ -6,6 +6,7 @@ import {
  ForbiddenError,
  RequestOptions,
  RESTDataSource,
+ WillSendRequestOptions,
  // RequestOptions
 } from '../RESTDataSource';
 
@@ -95,17 +96,30 @@ describe('RESTDataSource', () => {
 
  getPostsForUser(
  username: string,
- params: { filter: string; limit: number; offset: number },
+ params: {
+ filter: string;
+ limit: number;
+ offset: number;
+ optional?: string;
+ },
  ) {
  return this.get('posts', {
  params: {
  username,
  filter: params.filter,
  limit: params.limit.toString(),
  offset: params.offset.toString(),
+ // In the test, this is undefined and should not end up in the URL.
+ optional: params.optional,
  },
  });
  }
+
+ getPostsWithURLSearchParams(username: string) {
+ return this.get('posts2', {
+ params: new URLSearchParams([['username', username]]),
+ });
+ }
  })();
 
  nock(apiUrl)
@@ -118,11 +132,19 @@ describe('RESTDataSource', () => {
  })
  .reply(200);
 
+ nock(apiUrl)
+ .get('/posts2')
+ .query({
+ username: 'beyoncé',
+ })
+ .reply(200);
+
  await dataSource.getPostsForUser('beyoncé', {
  filter: 'jalapeño',
  limit: 10,
  offset: 20,
  });
+ await dataSource.getPostsWithURLSearchParams('beyoncé');
  });
 
  it('allows setting default query string parameters', async () => {
@@ -133,10 +155,8 @@ describe('RESTDataSource', () => {
  super(config);
  }
 
- override willSendRequest(request: RequestOptions) {
- const params = new URLSearchParams(request.params);
- params.set('apiKey', this.token);
- request.params = params;
+ override willSendRequest(request: WillSendRequestOptions) {
+ request.params.set('apiKey', this.token);
  }
 
  getFoo(id: string) {
@@ -155,7 +175,7 @@ describe('RESTDataSource', () => {
  const dataSource = new (class extends RESTDataSource {
  override baseURL = 'https://api.example.com';
 
- override willSendRequest(request: RequestOptions) {
+ override willSendRequest(request: WillSendRequestOptions) {
  request.headers = { ...request.headers, credentials: 'include' };
  }
 
@@ -177,7 +197,7 @@ describe('RESTDataSource', () => {
  super(config);
  }
 
- override willSendRequest(request: RequestOptions) {
+ override willSendRequest(request: WillSendRequestOptions) {
  request.headers = { ...request.headers, authorization: this.token };
  }