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

refactor: Refactor mergeConfig function to remove duplicate keys #5605

Closed
wants to merge 2 commits into from

Conversation

lei-mu
Copy link

@lei-mu lei-mu commented Mar 16, 2023

Refactor mergeConfig function to use uniqueArray for deduplication
This commit refactors the mergeConfig function to use the uniqueArray
function instead of an array concatenation and filtering method. This change
improves performance by reducing unnecessary iterations over large arrays, and
also simplifies the code. The resulting code is more efficient, easier to read,
and conforms better to modern JS standards.

@WillianAgostini
Copy link
Contributor

Hi @lei-mu
Great PR, I liked that with this change it was better to read the code.
I understood that the main point here is the increase in performance. Can you write a test case measuring this performance increase?

@DigitalBrainJS
Copy link
Collaborator

It is likely that a simple expression like

  utils.forEach(Object.keys(Object.assign({}, config1, config2)), function computeConfigValue(prop) {
    //...
  });

will have better performance in a shorter form.

v16.19.0
unique performance:
spread x 366,729 ops/sec ±3.45% (74 runs sampled)
assign x 2,321,281 ops/sec ±2.62% (81 runs sampled)
filter x 1,572,643 ops/sec ±0.52% (90 runs sampled)
Fastest is assign

Process finished with exit code 0

@lei-mu lei-mu closed this Sep 12, 2023
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.

None yet

3 participants