From 12e767e9e89ba1502080b2305406ad1bba8fbb7b Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Wed, 24 Mar 2021 00:17:01 -0500 Subject: [PATCH 1/4] feat(ajax): add `params` query string parameter configuration --- spec/observables/dom/ajax-spec.ts | 167 ++++++++++++++++++++++++++++++ src/internal/ajax/ajax.ts | 62 ++++++++--- src/internal/ajax/types.ts | 18 ++++ 3 files changed, 233 insertions(+), 14 deletions(-) diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts index 8d597b17e4..7ceffd1cc0 100644 --- a/spec/observables/dom/ajax-spec.ts +++ b/spec/observables/dom/ajax-spec.ts @@ -1366,6 +1366,173 @@ describe('ajax', () => { x-custom-header: test x-headers-are-fun: {"weird": "things"}`); }); + + describe('with params', () => { + it('should allow passing of search params as a dictionary', () => { + ajax({ + method: 'GET', + url: '/whatever', + params: { foo: 'bar', whatever: '123' }, + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?foo=bar&whatever=123'); + }); + + it('should allow passing of search params as an entries array', () => { + ajax({ + method: 'GET', + url: '/whatever', + params: [ + ['foo', 'bar'], + ['whatever', '123'], + ], + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?foo=bar&whatever=123'); + }); + + it('should allow passing of search params as a string', () => { + ajax({ + method: 'GET', + url: '/whatever', + params: '?foo=bar&whatever=123', + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?foo=bar&whatever=123'); + }); + + it('should allow passing of search params as a URLSearchParams object', () => { + const params = new URLSearchParams(); + params.set('foo', 'bar'); + params.set('whatever', '123'); + ajax({ + method: 'GET', + url: '/whatever', + params: '?foo=bar&whatever=123', + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?foo=bar&whatever=123'); + }); + + it('should not screw things up if there is an existing search string in the url passed', () => { + ajax({ + method: 'GET', + url: '/whatever?jays_face=is+a+param&lol=haha', + params: { foo: 'bar', whatever: '123' }, + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?jays_face=is+a+param&lol=haha&foo=bar&whatever=123'); + }); + + it('should overwrite existing args from existing search strings in the url passed', () => { + ajax({ + method: 'GET', + url: '/whatever?terminator=2&uncle_bob=huh', + params: { uncle_bob: '...okayyyyyyy', movie_quote: 'yes' }, + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?terminator=2&uncle_bob=...okayyyyyyy&movie_quote=yes'); + }); + + it('should properly encode values', () => { + ajax({ + method: 'GET', + url: '/whatever', + params: { 'this is a weird param name': '?#* value here rofl !!!' }, + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?this+is+a+weird+param+name=%3F%23*+value+here+rofl+%21%21%21'); + }); + + it('should handle dictionaries that have numbers, booleans, and arrays of numbers, strings or booleans', () => { + ajax({ + method: 'GET', + url: '/whatever', + params: { a: 123, b: true, c: ['one', 'two', 'three'], d: [1, 3, 3, 7], e: [true, false, true] }, + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?a=123&b=true&c=one%2Ctwo%2Cthree&d=1%2C3%2C3%2C7&e=true%2Cfalse%2Ctrue'); + }); + + it('should handle entries that have numbers, booleans, and arrays of numbers, strings or booleans', () => { + ajax({ + method: 'GET', + url: '/whatever', + params: [ + ['a', 123], + ['b', true], + ['c', ['one', 'two', 'three']], + ['d', [1, 3, 3, 7]], + ['e', [true, false, true]], + ], + }).subscribe(); + + const mockXHR = MockXMLHttpRequest.mostRecent; + + mockXHR.respondWith({ + status: 200, + responseText: JSON.stringify({ whatever: 'I want' }), + }); + + expect(mockXHR.url).to.equal('/whatever?a=123&b=true&c=one%2Ctwo%2Cthree&d=1%2C3%2C3%2C7&e=true%2Cfalse%2Ctrue'); + }); + }); }); // Some of the older versions of node we test on don't have EventTarget. diff --git a/src/internal/ajax/ajax.ts b/src/internal/ajax/ajax.ts index f67aab65db..cd22ed4435 100644 --- a/src/internal/ajax/ajax.ts +++ b/src/internal/ajax/ajax.ts @@ -275,15 +275,53 @@ const LOAD = 'load'; export function fromAjax(config: AjaxConfig): Observable> { return new Observable((destination) => { + // Here we're pulling off each of the configuration arguments + // that we don't want to add to the request information we're + // passing around. + const { params, body: configuredBody, headers: configuredHeaders, ...remainingConfig } = config; + + let { url } = remainingConfig; + if (!url) { + throw new TypeError('url is required'); + } + + if (params) { + let searchParams: URLSearchParams; + if (url.includes('?')) { + // If the user has passed a URL with a querystring already in it, + // we need to combine them. So we're going to split it. There + // should only be one `?` in a valid URL. + const parts = url.split('?'); + if (2 < parts.length) { + throw new TypeError('invalid url'); + } + // Add the passed params to the params already in the url provided. + searchParams = new URLSearchParams(parts[1]); + // params is converted to any because the runtime is *much* more permissive than + // the types are. + new URLSearchParams(params as any).forEach((value, key) => searchParams.set(key, value)); + // We have to do string concatenation here, because `new URL(url)` does + // not like relative URLs like `/this` without a base url, which we can't + // specify, nor can we assume `location` will exist, because of node. + url = parts[0] + '?' + searchParams; + } else { + // There is no pre-existing querystring, so we can just use URLSearchParams + // to convert the passed params into the proper format and encodings. + // params is converted to any because the runtime is *much* more permissive than + // the types are. + searchParams = new URLSearchParams(params as any); + url = url + '?' + searchParams; + } + } + // Normalize the headers. We're going to make them all lowercase, since // Headers are case insenstive by design. This makes it easier to verify // that we aren't setting or sending duplicates. const headers: Record = {}; - const requestHeaders = config.headers; - if (requestHeaders) { - for (const key in requestHeaders) { - if (requestHeaders.hasOwnProperty(key)) { - headers[key.toLowerCase()] = requestHeaders[key]; + if (configuredHeaders) { + for (const key in configuredHeaders) { + if (configuredHeaders.hasOwnProperty(key)) { + headers[key.toLowerCase()] = configuredHeaders[key]; } } } @@ -301,8 +339,8 @@ export function fromAjax(config: AjaxConfig): Observable> { // Allow users to provide their XSRF cookie name and the name of a custom header to use to // send the cookie. - const { withCredentials, xsrfCookieName, xsrfHeaderName } = config; - if ((withCredentials || !config.crossDomain) && xsrfCookieName && xsrfHeaderName) { + const { withCredentials, xsrfCookieName, xsrfHeaderName } = remainingConfig; + if ((withCredentials || !remainingConfig.crossDomain) && xsrfCookieName && xsrfHeaderName) { const xsrfCookie = document?.cookie.match(new RegExp(`(^|;\\s*)(${xsrfCookieName})=([^;]*)`))?.pop() ?? ''; if (xsrfCookie) { headers[xsrfHeaderName] = xsrfCookie; @@ -311,7 +349,7 @@ export function fromAjax(config: AjaxConfig): Observable> { // Examine the body and determine whether or not to serialize it // and set the content-type in `headers`, if we're able. - const body = extractContentTypeAndMaybeSerializeBody(config.body, headers); + const body = extractContentTypeAndMaybeSerializeBody(configuredBody, headers); const _request: AjaxRequest = { // Default values @@ -323,20 +361,16 @@ export function fromAjax(config: AjaxConfig): Observable> { responseType: 'json' as XMLHttpRequestResponseType, // Override with passed user values - ...config, + ...remainingConfig, // Set values we ensured above + url, headers, body, }; let xhr: XMLHttpRequest; - const { url } = _request; - if (!url) { - throw new TypeError('url is required'); - } - // Create our XHR so we can get started. xhr = config.createXHR ? config.createXHR() : new XMLHttpRequest(); diff --git a/src/internal/ajax/types.ts b/src/internal/ajax/types.ts index 47d653852e..ac9848a75f 100644 --- a/src/internal/ajax/types.ts +++ b/src/internal/ajax/types.ts @@ -195,4 +195,22 @@ export interface AjaxConfig { * be emitted from the resulting observable. */ includeUploadProgress?: boolean; + + /** + * Query string parameters to add to the URL in the request. + * This will require a polyfill for `URL` and `URLSearchParams` in Internet Explorer! + * + * Accepts either a query string, a `URLSearchParams` object, a dictionary of key/value pairs, or an + * array of key/value entry tuples. (Essentially, it takes anything that `new URLSearchParams` would normally take). + * + * If, for some reason you have a query string in the `url` argument, this will append to the query string in the url, + * but it will also overwrite the value of any keys that are an exact match. In other words, a url of `/test?a=1&b=2`, + * with params of `{ b: 5, c: 6 }` will result in a url of roughly `/test?a=1&b=5&c=6`. + */ + params?: + | string + | URLSearchParams + | Record + | [string, string | number | boolean | string[] | number[] | boolean[]][] + | undefined; } From fa9415e9c25d3e7819c238b5d6da5e825434811b Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Wed, 24 Mar 2021 00:28:18 -0500 Subject: [PATCH 2/4] chore: update golden files --- api_guard/dist/types/ajax/index.d.ts | 1 + integration/side-effects/snapshots/esm/ajax.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api_guard/dist/types/ajax/index.d.ts b/api_guard/dist/types/ajax/index.d.ts index 8013eb1d16..565c045a95 100644 --- a/api_guard/dist/types/ajax/index.d.ts +++ b/api_guard/dist/types/ajax/index.d.ts @@ -9,6 +9,7 @@ export interface AjaxConfig { includeDownloadProgress?: boolean; includeUploadProgress?: boolean; method?: string; + params?: string | URLSearchParams | Record | [string, string | number | boolean | string[] | number[] | boolean[]][] | undefined; password?: string; progressSubscriber?: PartialObserver; responseType?: XMLHttpRequestResponseType; diff --git a/integration/side-effects/snapshots/esm/ajax.js b/integration/side-effects/snapshots/esm/ajax.js index 8b13789179..5d63e0559f 100644 --- a/integration/side-effects/snapshots/esm/ajax.js +++ b/integration/side-effects/snapshots/esm/ajax.js @@ -1 +1 @@ - +import "tslib"; From e59441fb42876ecc918010dbd272bf9de4a17dd2 Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Sun, 28 Mar 2021 17:38:58 -0500 Subject: [PATCH 3/4] chore: fix test --- spec/observables/dom/ajax-spec.ts | 2 +- src/internal/ajax/types.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/observables/dom/ajax-spec.ts b/spec/observables/dom/ajax-spec.ts index 7ceffd1cc0..5d0e982831 100644 --- a/spec/observables/dom/ajax-spec.ts +++ b/spec/observables/dom/ajax-spec.ts @@ -1429,7 +1429,7 @@ x-headers-are-fun: {"weird": "things"}`); ajax({ method: 'GET', url: '/whatever', - params: '?foo=bar&whatever=123', + params, }).subscribe(); const mockXHR = MockXMLHttpRequest.mostRecent; diff --git a/src/internal/ajax/types.ts b/src/internal/ajax/types.ts index ac9848a75f..fac9537ade 100644 --- a/src/internal/ajax/types.ts +++ b/src/internal/ajax/types.ts @@ -211,6 +211,5 @@ export interface AjaxConfig { | string | URLSearchParams | Record - | [string, string | number | boolean | string[] | number[] | boolean[]][] - | undefined; + | [string, string | number | boolean | string[] | number[] | boolean[]][]; } From afc0490ff02c7f5a777a02048109a8902345f99f Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Sun, 28 Mar 2021 18:20:32 -0500 Subject: [PATCH 4/4] chore: update api_guardian golden files --- api_guard/dist/types/ajax/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_guard/dist/types/ajax/index.d.ts b/api_guard/dist/types/ajax/index.d.ts index 565c045a95..e213206fbb 100644 --- a/api_guard/dist/types/ajax/index.d.ts +++ b/api_guard/dist/types/ajax/index.d.ts @@ -9,7 +9,7 @@ export interface AjaxConfig { includeDownloadProgress?: boolean; includeUploadProgress?: boolean; method?: string; - params?: string | URLSearchParams | Record | [string, string | number | boolean | string[] | number[] | boolean[]][] | undefined; + params?: string | URLSearchParams | Record | [string, string | number | boolean | string[] | number[] | boolean[]][]; password?: string; progressSubscriber?: PartialObserver; responseType?: XMLHttpRequestResponseType;