Skip to content

Comments

Refactor mergeConfig without utils.deepMerge (#2844)#3

Open
MitchLewis930 wants to merge 1 commit intopr_023_beforefrom
pr_023_after
Open

Refactor mergeConfig without utils.deepMerge (#2844)#3
MitchLewis930 wants to merge 1 commit intopr_023_beforefrom
pr_023_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_023


Note

Medium Risk
Touches core request config/headers merging behavior, which can subtly change how user overrides (especially headers and validateStatus) are applied across all requests.

Overview
Refactors mergeConfig and related merging utilities. Axios method aliases now use mergeConfig directly, and mergeConfig is rewritten to rely on a new utils.isPlainObject plus an enhanced utils.merge (array cloning + plain-object-only deep merge), removing utils.deepMerge entirely.

This changes config merging semantics: adds/handles more known config keys, treats validateStatus as a direct-merge key (allowing explicit undefined/null overrides), preserves null/undefined header values rather than deleting them during dispatchRequest, and broadens merging to include custom/default config keys with cloning to avoid shared references. Tests are expanded/updated accordingly (including removing deepMerge specs and adding coverage for header reset and validateStatus behavior).

Written by Cursor Bugbot for commit 0d69a79. This will update automatically on new commits. Configure here.

* Adding failing test

* Fixing axios#2587 default custom config persisting

* Adding Concat keys and filter duplicates

* Fixed value from CPE

* update for review feedbacks

* no deepMerge

* only merge between plain objects

* fix rename

* always merge config by mergeConfig

* extract function mergeDeepProperties

* refactor mergeConfig with all keys, and add special logic for validateStatus

* add test for resetting headers

* add lots of tests and fix a bug

* should not inherit `data`

* use simple toString

* revert axios#1845

Co-authored-by: David Tanner <david.tanner@lifeomic.com>
Co-authored-by: Justin Beckwith <justin.beckwith@gmail.com>
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

}
);

// Remove header where value is null
Copy link

Choose a reason for hiding this comment

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

Null-valued headers no longer removed before sending requests

High Severity

The code that filtered out headers with null values was removed from dispatchRequest.js. Previously, setting a header to null would remove it from the request (useful for overriding default headers). Now, null-valued headers are passed to XMLHttpRequest.setRequestHeader(), which stringifies null to the string "null" and sends it in the request. This breaks the documented pattern of using null to unset default headers and will cause unexpected header values in production requests.

Fix in Cursor Fix in Web

config[prop] = config1[prop];
}
});
utils.forEach(otherKeys, mergeDeepProperties);
Copy link

Choose a reason for hiding this comment

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

Duplicate keys cause redundant processing in otherKeys loop

Low Severity

otherKeys is built by concatenating Object.keys(config1) and Object.keys(config2) without deduplication. When both configs have the same custom property, mergeDeepProperties will be called twice for that key. Consider deduplicating with Array.from(new Set(...)) or filtering duplicates.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants