Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Use spread properties instead of Object.assign()/deepmerge #859

Merged
merged 3 commits into from
May 8, 2018
Merged

Use spread properties instead of Object.assign()/deepmerge #859

merged 3 commits into from
May 8, 2018

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented May 8, 2018 •
edited
Loading

Now that we require Node 8.3+, we can use spread properties instead of:

  • Object.assign()
  • deepmerge in cases where we just need a shallow merge.

http://node.green/#ES2018-features-object-rest-spread-properties

@edmorley
Replace shallow deepmerge usages with spread properties
de65cde 
Now that we require Node 8.3+, we can use spread properties instead
of deepmerge in cases where we just need a shallow merge.

http://node.green/#ES2018-features-object-rest-spread-properties
@edmorley edmorley self-assigned this May 8, 2018
@eliperelman
Copy link
Member

eliperelman commented May 8, 2018 •
edited
Loading

I haven't yet looked through the code, but does this also account for Object.assigns we could eliminate as well?

@edmorley
Copy link
Member Author

edmorley commented May 8, 2018

Ah I forgot to check those too

eliperelman
eliperelman reviewed May 8, 2018
View reviewed changes
Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

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

You'll need to wrap arrow function returned objects with parentheses or it will be ambiguous with a function body.

@edmorley edmorley requested a review from eliperelman May 8, 2018 19:03
eliperelman
eliperelman approved these changes May 8, 2018
View reviewed changes
Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

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

Everything looks good to me.

@edmorley edmorley changed the title Replace shallow deepmerge usages with spread properties Use spread properties instead of Object.assign or deepmerge May 8, 2018
@edmorley edmorley changed the title Use spread properties instead of Object.assign or deepmerge Use spread properties instead of Object.assign()/deepmerge May 8, 2018
@edmorley edmorley merged commit 33fc8ad into neutrinojs:master May 8, 2018
@edmorley edmorley deleted the deepmerge-to-spread branch May 8, 2018 19:43
@edmorley
Copy link
Member Author

edmorley commented May 8, 2018

Bah I used squash and merge and intended to update the commit message to reflect the new summary/description of this PR (ie Object.assign() too), but only updated the commit message body not the first line. Not worth force pushing to fix, but annoying it's not accurate 🙁.

@edmorley edmorley added this to the v9 milestone Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eliperelman eliperelman eliperelman approved these changes

Assignees

@edmorley edmorley

Labels
breaking change cleanup
Milestone
Neutrino 9
Development

Successfully merging this pull request may close these issues.
2 participants
@edmorley @eliperelman