Core(utils): ajax - remove m prefix and improve typing by dmlvr · Pull Request #35290 · DevExpress/DevExtreme

dmlvr · 2026-09-22T10:42:18Z

No description provided.

Rename js/__internal/core/utils/m_ajax.ts and m_ajax_utils.ts, updating the two public re-export shims and the jQuery ajax integration. No content change - the rename is kept as its own commit so git and GitHub preserve the file history. The renamed files now fall under the strict eslint ruleset, so this commit alone does not lint clean; the follow-up commit fixes that. Committed with --no-verify for that reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

FormData requests without upload lose their payload during parameter conversion and receive an incorrect content type.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Preserve FormData payload and multipart Content-Type handling · New

What changed in this PR

Migrates AJAX utilities from m_-prefixed modules to typed TypeScript implementations.

Changes:

Renamed AJAX utility imports and exports.
Added TypeScript typings for requests, headers, uploads, and XHR handling.
Updated jQuery integration typing.

File	Description
`packages/devextreme/js/core/utils/ajax.js`	Updated AJAX implementation import.
`packages/devextreme/js/core/utils/ajax_utils.js`	Updated utility re-export path.
`packages/devextreme/js/__internal/integration/jquery/ajax.ts`	Updated import and jQuery options typing.
`packages/devextreme/js/__internal/core/utils/m_ajax_utils.ts`	Removed obsolete implementation.
`packages/devextreme/js/__internal/core/utils/ajax.ts`	Added typed request handling.
`packages/devextreme/js/__internal/core/utils/ajax_utils.ts`	Added typed AJAX helper implementation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

jQuery integration still serializes FormData when upload is omitted.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

jQuery integration incorrectly serializes FormData requests · New

Resolved since last review (1)

Preserve FormData payload and multipart Content-Type handling

Copilot

Copilot review overview

🟡 Changes recommended

FormData handling remains broken in the Angular transport and for cross-window FormData instances.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Use cross-realm-safe FormData detection · New

Resolved since last review (1)

jQuery integration incorrectly serializes FormData requests

Copilot

Copilot review overview

🟡 Changes recommended

The Angular HTTP adapter still converts no-upload FormData requests into an empty URL-encoded payload.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Angular adapter mishandles FormData when upload is omitted · New

Resolved since last review (1)

Use cross-realm-safe FormData detection

Copilot

Copilot review overview

🟡 Changes recommended

A moderate FormData mutation issue remains, and Angular coverage is requested.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Add Angular test for FormData requests without upload · New

Resolved since last review (1)

Angular adapter mishandles FormData when upload is omitted

Copilot

Copilot review overview

🔵 Needs a closer look

Resolve Angular cross-realm FormData serialization and GET FormData query handling.

Review effort: Lite
Findings: None

Resolved since last review (1)

Add Angular test for FormData requests without upload

EugeniyKiyashko

I verified this by running the old and the new ajax_utils side by side in a browser - both files transpiled as they are, with only the three imports stubbed out. 30 different options shapes went through getRequestHeaders + getRequestOptions, 11 more through getJsonpCallbackName, plus the helpers on their own. 5 of those 51 cases differ.

Four of the five are the FormData fix this PR is about: the old code threw TypeError: Illegal invocation in all four, so those calls could not have worked before. The fifth difference has nothing to do with FormData and looks unintended - comment below.

I also ran the jQuery layer against jQuery 4.0.0 with real requests. There is one case that works today and changes behaviour after this PR. That is the comment I would most like a reply to.

Everything else came out identical, including: the _ timestamp parameter and where it ends up, query string joining, Content-Type in every shape, Accept, X-Requested-With, string and array data, undefined / null / function / Date / nested values, an empty url, all 11 jsonp shapes, getMethod, getAcceptHeader, isCrossDomain and evalScript. I checked each individual replacement in ajax.ts separately as well - ?? null on the upload handlers, options.timeout ?? 0, Deferred() instead of new Deferred(), Object.entries(headers) - and none of them changes anything at runtime.

Two notes that are not about the code:

GitHub did not recognise m_ajax_utils.ts -> ajax_utils.ts as a rename. The files API reports one file added (245 lines) and another removed (202); for ajax.ts the rename was recognised. So in the default "Files changed" view the whole of ajax_utils.ts reads as new code and the real diff is not visible there at all. Worth reviewing this one commit by commit.
The FormData fix is a user visible behaviour change across three layers, inside a PR titled as a rename and a typing pass. The commit message explains it well, but it is easy to miss when release notes are written. Worth deciding whether it deserves its own card.

EugeniyKiyashko · 2026-09-22T19:59:17Z

-        return jQuery.ajax(options);
+      // NOTE: FormData goes to the base implementation. Only jQuery 4 stops running it
+      // through jQuery.param, and we still support jQuery from 1.10.
+      if (!options.responseType && !options.upload && !isFormData(options.data)) {


On jQuery 4 this case works today. jQuery ships a prefilter (jquery.js:9181 in 4.0.0) that sets processData and contentType to false when the data is a FormData, so the request already goes out as proper multipart, and jQuery works out the response type from the response Content-Type. I ran it against a JSON endpoint: sendRequest({ url, method: 'POST', data: formData }) resolves with a parsed object today.

After this change the same call goes to the base implementation. There postProcess has no dataType to switch on, falls into default: and resolves with xhr.responseText - a string. The caller also loses the jqXHR, jQuery's global ajax events and its converters. So in the same application a POST with a plain object keeps resolving with an object, while a POST with a FormData starts resolving with a string.

The hole you are closing is real, though. That prefilter is s.data instanceof window.FormData, so a FormData created inside an iframe does not match it, and jQuery then sends a multipart body with its default urlencoded Content-Type. I reproduced that as well.

Passing the flags explicitly fixes both that and jQuery 3, without leaving jQuery.ajax:

if (!options.responseType && !options.upload) { const settings = isFormData(options.data) ? { ...options, processData: false, contentType: false } : options; return jQuery.ajax(settings as JQuery.AjaxSettings); }

I checked all three cases against jQuery 4.0.0 with real requests: the iframe FormData and the normal one both go out as multipart/form-data with a boundary, and all of them still resolve with a parsed object.

EugeniyKiyashko · 2026-09-22T19:59:17Z

+const paramsConvert = (params: Record<string, unknown>): string => {
+  const result: string[] = [];
+
+  Object.entries(params).forEach(([name, rawValue]) => {


This is the fifth difference I mentioned, and the only one that is not about FormData.

With data built as Object.create({ inherited: 'yes' }) plus an own own: 1, the old code sent own=1&_=...&inherited=yes and the new one sends own=1&_=.... Properties that come from the prototype are now silently dropped. The same applies to options.xhrFields in ajax.ts.

The commit message does call this out, and own properties only is the better rule - the Angular adapter already uses Object.keys. So this is really a request for one test: right now nothing in the suite would notice if this changed again, in either direction.

added a test

EugeniyKiyashko · 2026-09-22T19:59:17Z

+  if (!paramsAlreadyString && !options.cache) {
+    params = params || {};
+    (params as Record<string, unknown>)._ = Date.now();
+  }
+
+  if (params && !options.upload && !isFormData(params)) {


Two small things in this block. Neither is a regression, both were hidden by the crash before.

The FormData check sits below the _ timestamp assignment, so with an explicit cache: false we still write a _ property onto the caller's FormData object. It never reaches the request, so it is a dead write that also modifies an object we do not own. Moving the check above the block would cover both. With the default cache value this block does not run at all, so only an explicit cache: false is affected.

And for a GET request with a FormData, nothing is sent now: the whole block is skipped, the FormData is returned as the body, and XHR ignores a body on GET. Neither the fields nor the _ parameter reach the URL. It used to throw, so this is still an improvement - but it is worth making it a deliberate decision rather than a side effect.

fixed and added some tests 1 and 2

EugeniyKiyashko · 2026-09-22T19:59:17Z

+    const xhr = this.requests[0];
+
+    assert.strictEqual(xhr.requestBody, formData, 'FormData reaches send untouched');
+    assert.notEqual(xhr.requestHeaders['Content-Type'], 'application/x-www-form-urlencoded;charset=utf-8',


notEqual passes for any value other than the urlencoded one, which is weaker than what can be proved here.

The fake XHR decides by data instanceof globalObject.FormData (nise 6.1.5, lib/fake-xhr/index.js:910). In this test that check succeeds, so no Content-Type is set at all and assert.strictEqual(xhr.requestHeaders['Content-Type'], undefined) holds.

In the iframe test below, the same check fails and the fake XHR sets text/plain;charset=utf-8 itself - so the assertion there passes for a reason that has nothing to do with what the test is about. Asserting that exact value, or dropping the header assertion there and keeping the requestBody one which is the real point, would be clearer.

fixed, used strictEqual

EugeniyKiyashko · 2026-09-22T19:59:17Z

-const hasContent = function (status) {
-  return status !== 204;
-};
+type AjaxRequestResult = DeferredObj<unknown> & { abort?: () => void };


deferred.d.ts declares promise(): Promise<T>, and the returned object really does not have resolve / reject - that is the point of calling .promise(). This type says it does, and the as unknown as cast on line 88 is the symptom.

Promise<unknown> & { abort?: () => void } describes the runtime object correctly, reduces the cast to a single one, and matches the Result type the Angular adapter already uses.

EugeniyKiyashko · 2026-09-22T19:59:17Z

+    ? 'application/x-www-form-urlencoded;charset=utf-8'
+    : undefined;
+
+  return options.contentType


Not something to change in this PR, but worth knowing now that the type advertises false: contentType: false does not suppress the header. For a POST with data it falls through to the urlencoded default. I confirmed both the old and the new version behave this way, so it is not new.

The only place in the repo that passes false (odata/utils.ts:284) does so on GET, where there is no default anyway - so the option works by coincidence rather than by design. Might deserve a separate card.

Agreed, and agreed it does not belong in this PR. Recorded as a separate item.

EugeniyKiyashko · 2026-09-22T19:59:17Z

+
+  const random = Math.random().toString().replace(/\D/g, '');
+  const callbackName = options.jsonpCallback || `dxCallback${Date.now()}_${random}`;
+  const callbackParameter = (options.jsonp || 'callback') as string;


The cast hides a case the type allows: with jsonp: true the callback parameter name becomes the string "true". I confirmed both the old and the new version do this, so it is not new.

jsonp?: string | false, or a typeof check, would remove the cast instead of hiding the case.

EugeniyKiyashko · 2026-09-22T19:59:17Z

+  try {
+    urlAnchor.href = url as string;
+
+    // eslint-disable-next-line no-self-assign


The // NOTE: IE11 comment that used to sit here went away with the refactoring. It was the only hint why this line exists. Either bring it back, or remove the self-assignment together with evidence that nothing depends on it any more.

comment returned

Copilot

Copilot review overview

🟡 Changes recommended

Resolve the outstanding FormData cache-busting issues in the Angular and core AJAX paths.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Avoid mutating FormData during cache busting · New
Add cache buster for GET requests with FormData · New

@ts-expect-error

ajax_utils.ts Introduce AjaxRequestOptions, AjaxRequestHeaders, AjaxRequestData, AjaxUploadHandlers and AjaxRequestTarget, and type every helper against them. The option shape is drawn from the call sites that already exist (file uploader, map providers, OData, file manager, aspnet), not invented: jsonp stays string | boolean because jQuery lets it be false, headers keep unknown values because callers forward whatever their own options held, and responseType stays a plain string because the file manager passes one. The for-in loops became Object.entries/Object.keys - guard-for-in and no-restricted-syntax are on for renamed files. That drops inherited enumerable properties; for the params object it also turns `continue` into an early return. That change uncovered a latent bug, so FormData is now passed through untouched when the upload option is omitted, and no longer forces the urlencoded content type. Previously such a request never worked at all: for-in over a FormData enumerates its prototype methods, paramsConvert saw them as functions and called them unbound, and the request died with "TypeError: Illegal invocation". Switching to Object.entries would have turned that crash into a silently malformed body of just the cache-buster, which is worse. The path with upload set - the one the file uploader and the file manager actually use - is untouched. isFormData deliberately compares Object.prototype.toString instead of using instanceof, the way core/utils/type already identifies built-ins. A FormData built in another window - an iframe, or whatever getWindow() points at - belongs to a different realm and fails instanceof, which would have sent it through paramsConvert as an empty body. createScript now uses Object.assign over a Pick of HTMLScriptElement instead of copying arbitrary keys, and removeScript guards parentNode - it is only ever called on a node that was just appended to head. The `||` fallbacks are kept, with a file-level eslint-disable: contentType: false is the documented way to suppress the header, and an empty url, method or dataType has always fallen back to the default. The module is re-exported from the deprecated but still public core/utils/ajax_utils, so untyped JS can pass those values. ajax.ts Type the xhr as XMLHttpRequest & { customStatus?: string } and give the helpers real signatures. `new Deferred()` became `Deferred()`, which is what the function already did internally - that removes the @ts-expect-error and the `any` it was leaking. setHttpTimeout returns a timeout id or undefined instead of relying on `timeout && setTimeout(...)`, and timeoutId is assigned once at declaration to satisfy init-declarations without tripping no-undef-init. `options.cache === undefined` is deliberately not ??=: only undefined means "not set" here, a null from untyped JS has always been read as "no caching". AjaxRequestResult is Promise<unknown> & { abort?: () => void }, which is what promise() actually returns - the object has no resolve or reject - and it reduces the double cast to a single one. The return type stays `any` on purpose. An XHR payload is untyped by nature, and sendRequest is injectable - the jQuery integration replaces it with one returning a jqXHR, which shares no structure with our Deferred. Every consumer has always received `any`; narrowing it would only push casts into odata/utils, remote_provider and the Azure map provider. integration/jquery/ajax.ts A FormData request keeps going to jQuery.ajax, now with processData and contentType set to false. jQuery 4 sets those itself, but only for a FormData of its own realm, and jQuery 3 - which integration/jquery.ts still accepts - has no such prefilter. Handing the request to the base implementation instead would have changed what callers get back: jQuery picks the response type from the response Content-Type, our postProcess has no dataType to switch on and resolves with responseText, so the same application would see an object for a plain object and a string for a FormData. Cast our options to JQuery.AjaxSettings. The shapes differ in beforeSend - jQuery hands the callback its own jqXHR, we hand over a real XMLHttpRequest - and that only surfaced once the options were typed. FormData requests now go to the base implementation instead of jQuery.ajax, so the fix above holds on every jQuery we support. Only jQuery 4 stops running FormData through jQuery.param: it ships an ajaxPrefilter that sets processData and contentType to false for it. jQuery 3.7.1 does not mention FormData at all, and jQuery.param there walks it with for-in and dies the same way our own code did - while integration/jquery.ts accepts anything from 1.10 up. core/utils/ajax_utils.js, devextreme-angular/src/http/ajax.ts The Angular DxHttpModule adapter builds on these helpers and undid the FormData fix: it re-added the urlencoded Content-Type whenever the helper left it unset, and its makeBody then serialized the body with Object.keys, reducing a FormData to just the cache-buster. Export isFormData from the public shim - wrappers can only import from there - and skip both steps for it. The makeBody guard has to come before the headers[CONTENT_TYPE] lookup, since the line it now bypasses is what guaranteed that header was set. utils.ajax.tests.js, devextreme-angular/tests/src/http/ajax.spec.ts Cover the FormData case, which had no test at all, including a cross-realm FormData built in an iframe. The Angular adapter gets its own spec, since its header and body logic is independent of the core helpers and the existing specs only exercise FormData together with upload. Reverting either adapter guard makes that spec fail on both of its assertions. Verified: eslint clean on every touched file, build:ts:internal green, and QUnit green on utils.ajax.tests.js (33 tests, 131/131, with and without jQuery), odataStore (251/251), fileManagerParts/remoteProvider (41/41) and fileUploader (244/244). devextreme-angular builds and its suites pass with the cache skipped: 164 client, 6 server, 2 hydration, no failures (165 client after the new spec). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Angular cross-realm FormData handling remains unresolved.

Review effort: Lite
Findings: None

Resolved since last review (2)

Add cache buster for GET requests with FormData
Avoid mutating FormData during cache busting

dmlvr self-assigned this Sep 22, 2026

Copilot AI lite review requested due to automatic review settings September 22, 2026 10:42

dmlvr added the 26_2 label Sep 22, 2026

Copilot started reviewing on behalf of dmlvr September 22, 2026 10:42

View session