Skip to content

Simplify cachedMerge - #7013

Merged
sokra merged 1 commit into
masterfrom
refactor/cachedmerge
Apr 11, 2018
Merged

sokra merged 1 commit into
masterfrom
refactor/cachedmerge

Conversation

@ooflorent

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

refactoring

Did you add tests for your changes?

no

If relevant, link to documentation update:

N/A

Summary

cachedMerge is only called with 2 two arguments. It doesn't need to recursive.

Does this PR introduce a breaking change?

no

Comment thread lib/util/cachedMerge.js
}
const cachedMerge = innerCache.get(second);
if (cachedMerge !== undefined) return cachedMerge;
const prevMerge = innerCache.get(second);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Avoid shadowing

@webpack-bot

Copy link
Copy Markdown
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

Comment thread lib/util/cachedMerge.js
const cachedMerge = (first, ...args) => {
if (args.length === 0) return first;
if (args.length > 1) {
return cachedMerge(first, cachedMerge(...args));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It was so cool...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sokra
sokra merged commit a41974d into master Apr 11, 2018
@sokra

sokra commented Apr 11, 2018

Copy link
Copy Markdown
Member

Thanks

@sokra
sokra deleted the refactor/cachedmerge branch April 11, 2018 06:29
@mohsen1 mohsen1 mentioned this pull request Apr 11, 2018
27 tasks done
mhegazy added a commit to mhegazy/webpack that referenced this pull request Apr 23, 2018
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.

5 participants