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

feat(ajax): add params query string parameter configuration #6174

Merged
merged 4 commits into from
Mar 28, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 167 additions & 0 deletions spec/observables/dom/ajax-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,173 @@ describe('ajax', () => {
x-custom-header: test
x-headers-are-fun: <whatever/> {"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();
benlesh marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand Down
62 changes: 48 additions & 14 deletions src/internal/ajax/ajax.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,15 +275,53 @@ const LOAD = 'load';

export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything inside of this conditional is the bulk of what was added. Please notice that it's mostly comments, and not actually a lot of code. Also worth noticing that URLSearchParams is only ever accessed inside of this conditional.

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<string, any> = {};
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];
}
}
}
Expand All @@ -301,8 +339,8 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {

// 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;
Expand All @@ -311,7 +349,7 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {

// 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
Expand All @@ -323,20 +361,16 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
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();

Expand Down
18 changes: 18 additions & 0 deletions src/internal/ajax/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <em>This will require a polyfill for `URL` and `URLSearchParams` in Internet Explorer!</em>
*
* 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[]>
| [string, string | number | boolean | string[] | number[] | boolean[]][]
| undefined;
benlesh marked this conversation as resolved.
Show resolved Hide resolved
}