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

perf: update deepCopy #4364

Merged
merged 5 commits into from
Oct 18, 2022
Merged

Conversation

cstuncsik
Copy link
Contributor

@cstuncsik cstuncsik commented Oct 17, 2022

JSON.parse(JSON.stringify(object)) has a very low performance

check the benchmarks here:
http://jsben.ch/WRTkU

@linear
Copy link

linear bot commented Oct 17, 2022

N8N-5067

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Oct 17, 2022
@mieky
Copy link
Member

mieky commented Oct 18, 2022

Thumbs up for the effort, love to see you take this on! 💯

The implementation seems very quick indeed. I would perhaps consider sticking with lodash's cloneDeep if only for the maintainability, and the fact we're using lodash in many places already. Importing it from within workflow does feel a bit funny, although I acknowledge it be the most practical way to go.

@cstuncsik
Copy link
Contributor Author

@mieky in frontend we actually want to get rid of lodash completely. I agree the place is not the best, in bigger projects I usually have a separate package for utility and helper functions but for now I'm ok with workflow

mutdmour
mutdmour previously approved these changes Oct 18, 2022
@cstuncsik cstuncsik merged commit 1aa21ed into master Oct 18, 2022
@cstuncsik cstuncsik deleted the n8n-5067-have-a-proper-javascript-deepcopy-for branch October 18, 2022 11:33
@n8n-assistant n8n-assistant bot added the Upcoming Release Will be part of the upcoming release label Oct 18, 2022
@janober
Copy link
Member

janober commented Oct 21, 2022

Got released with n8n@0.199.0

@janober janober removed the Upcoming Release Will be part of the upcoming release label Oct 21, 2022
valya pushed a commit to valya/n8n that referenced this pull request Nov 8, 2022
* perf: update deepCopy

* fix: using deepCopy in core and cli packages

* fix: using deepCopy in editor

* chore: formatting

* fix: some micro optimisation in deepCopy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants