fix(ajax): crossDomain flag deprecated and properly reported to consumers (#6710)

* fix(ajax): crossDomain flag deprecated and properly reported to consumers

Fixes an issue where the `crosDomain` flag was being incorrectly reported to users via error objects and response objects as defaulting to `true`, when it was in fact defaulting to `false`.

Deprecates the `crossDomain` flag in favor of allowing the configuration of the request and the browser to dictate whether or not a preflight request is necessary. Adds deprecation messages with advice about how to force CORS preflights. Ultimately, the boolean flag does not make sense, as there are a lot of factors that dictate CORS preflight use and they may vary by browser/environment.

Resolves #6663

* chore: Update side effects file

* chore: update side effects files

Author: benlesh

integration/side-effects/snapshots/esm/ajax.js

@@ -1 +1 @@
-import "tslib";
+

package-lock.json

@@ -180,7 +180,7 @@
     "npm-run-all": "4.1.2",
     "opn-cli": "3.1.0",
     "platform": "1.3.5",
-    "prettier": "^2.0.5",
+    "prettier": "^2.5.1",
     "promise": "8.0.1",
     "rollup": "0.66.6",
     "rollup-plugin-alias": "1.4.0",

package.json

@@ -208,6 +208,33 @@ describe('ajax', () => {
     });
   });
 
+  it('should NOT set the X-Requested-With if crossDomain is true', () => {
+    ajax({
+      url: '/test/monkey',
+      method: 'GET',
+      crossDomain: true,
+    }).subscribe();
+
+    const request = MockXMLHttpRequest.mostRecent;
+
+    expect(request.requestHeaders).to.not.have.key('x-requested-with');
+  });
+
+  it('should not alter user-provided X-Requested-With header, even if crossDomain is true', () => {
+    ajax({
+      url: '/test/monkey',
+      method: 'GET',
+      crossDomain: true,
+      headers: {
+        'x-requested-with': 'Custom-XMLHttpRequest',
+      },
+    }).subscribe();
+
+    const request = MockXMLHttpRequest.mostRecent;
+
+    expect(request.requestHeaders['x-requested-with']).to.equal('Custom-XMLHttpRequest');
+  });
+
   it('should not set default Content-Type header when no body is sent', () => {
     const obj: AjaxConfig = {
       url: '/talk-to-me-goose',
@@ -1025,7 +1052,7 @@ describe('ajax', () => {
       const request = {
         async: true,
         body: undefined,
-        crossDomain: true,
+        crossDomain: false,
         headers: {
           'x-requested-with': 'XMLHttpRequest',
         },
@@ -1153,7 +1180,7 @@ describe('ajax', () => {
       const request = {
         async: true,
         body: undefined,
-        crossDomain: true,
+        crossDomain: false,
         headers: {
           'x-requested-with': 'XMLHttpRequest',
         },

spec/observables/dom/ajax-spec.ts

@@ -292,14 +292,23 @@ const LOADSTART = 'loadstart';
 const PROGRESS = 'progress';
 const LOAD = 'load';
 
-export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
+export function fromAjax<T>(init: 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 { queryParams, body: configuredBody, headers: configuredHeaders, ...remainingConfig } = config;
+    const config = {
+      // Defaults
+      async: true,
+      crossDomain: false,
+      withCredentials: false,
+      method: 'GET',
+      timeout: 0,
+      responseType: 'json' as XMLHttpRequestResponseType,
+
+      ...init,
+    };
+
+    const { queryParams, body: configuredBody, headers: configuredHeaders } = config;
 
-    let { url } = remainingConfig;
+    let url = config.url;
     if (!url) {
       throw new TypeError('url is required');
     }
@@ -345,21 +354,23 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
       }
     }
 
+    const crossDomain = config.crossDomain;
+
     // Set the x-requested-with header. This is a non-standard header that has
     // come to be a de facto standard for HTTP requests sent by libraries and frameworks
     // using XHR. However, we DO NOT want to set this if it is a CORS request. This is
     // because sometimes this header can cause issues with CORS. To be clear,
     // None of this is necessary, it's only being set because it's "the thing libraries do"
     // Starting back as far as JQuery, and continuing with other libraries such as Angular 1,
     // Axios, et al.
-    if (!config.crossDomain && !('x-requested-with' in headers)) {
+    if (!crossDomain && !('x-requested-with' in headers)) {
       headers['x-requested-with'] = 'XMLHttpRequest';
     }
 
     // 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 } = remainingConfig;
-    if ((withCredentials || !remainingConfig.crossDomain) && xsrfCookieName && xsrfHeaderName) {
+    const { withCredentials, xsrfCookieName, xsrfHeaderName } = config;
+    if ((withCredentials || !crossDomain) && xsrfCookieName && xsrfHeaderName) {
       const xsrfCookie = document?.cookie.match(new RegExp(`(^|;\\s*)(${xsrfCookieName})=([^;]*)`))?.pop() ?? '';
       if (xsrfCookie) {
         headers[xsrfHeaderName] = xsrfCookie;
@@ -370,17 +381,9 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
     // and set the content-type in `headers`, if we're able.
     const body = extractContentTypeAndMaybeSerializeBody(configuredBody, headers);
 
-    const _request: AjaxRequest = {
-      // Default values
-      async: true,
-      crossDomain: true,
-      withCredentials: false,
-      method: 'GET',
-      timeout: 0,
-      responseType: 'json' as XMLHttpRequestResponseType,
-
-      // Override with passed user values
-      ...remainingConfig,
+    // The final request settings.
+    const _request: Readonly<AjaxRequest> = {
+      ...config,
 
       // Set values we ensured above
       url,
@@ -391,7 +394,7 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
     let xhr: XMLHttpRequest;
 
     // Create our XHR so we can get started.
-    xhr = config.createXHR ? config.createXHR() : new XMLHttpRequest();
+    xhr = init.createXHR ? init.createXHR() : new XMLHttpRequest();
 
     {
       ///////////////////////////////////////////////////
@@ -401,7 +404,7 @@ export function fromAjax<T>(config: AjaxConfig): Observable<AjaxResponse<T>> {
       // Otherwise the progress events will not fire.
       ///////////////////////////////////////////////////
 
-      const { progressSubscriber, includeDownloadProgress = false, includeUploadProgress = false } = config;
+      const { progressSubscriber, includeDownloadProgress = false, includeUploadProgress = false } = init;
 
       /**
        * Wires up an event handler that will emit an error when fired. Used

src/internal/ajax/ajax.ts

@@ -133,6 +133,14 @@ export interface AjaxConfig {
   /**
    * Whether or not to send the HTTP request as a CORS request.
    * Defaults to `false`.
+   *
+   * @deprecated Will be removed in version 8. Cross domain requests and what creates a cross
+   * domain request, are dictated by the browser, and a boolean that forces it to be cross domain
+   * does not make sense. If you need to force cross domain, make sure you're making a secure request,
+   * then add a custom header to the request or use `withCredentials`. For more information on what
+   * triggers a cross domain request, see the [MDN documentation](https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS#Requests_with_credentials).
+   * In particular, the section on [Simple Requests](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#Simple_requests) is useful
+   * for understanding when CORS will not be used.
    */
   crossDomain?: boolean;