fix(core): make deepCopy backward compatible
#4505
Conversation
`JSON.parse(JSON.stringify())` uses `.toJSON` when available. so should `deepCopy`
n8n-assistant
bot
added
core
* 🐛 Prevent double quotes on luxon datetimes * ⚡ Generalize solution
netroy
force-pushed
the
backward-compatible-deepcopy
branch
from
773948e
to
b0e2b18
Compare
…` on objects that have it
|
with LGTM. Please approve and merge at will. |
packages/workflow/src/utils.ts
Outdated
| @@ -28,7 +28,7 @@ export const deepCopy = <T extends ((object | Date) & { toJSON?: () => string }) | |||
| return clone as T; | |||
| } | |||
| // Object | |||
| const clone = {} as T; | |||
| const clone = Object.create(Object.getPrototypeOf(source)); | |||
There was a problem hiding this comment.
we might have the same issue here: the change makes semantic sense, but since we did not include the prototype in the cloned objects before, it's hard to know what side-effects this addition would have.
Can you please remove this?
we should definitely do this, just not in this PR.
There was a problem hiding this comment.
It works with Object.create(null) as well and passing the tests but in my perf testing it fails and works only with Object.create(Object.getPrototypeOf(source))
There was a problem hiding this comment.
maybe it's just assert.deepStrictEqual in nodejs
There was a problem hiding this comment.
this works in both places Object.create(Object.getPrototypeOf({}));
There was a problem hiding this comment.
Ok, it is just around 4% slower and still at the second place according to my perf test
| } | ||
| // Object | ||
| clone = {}; | ||
| const clone = Object.create(Object.getPrototypeOf({})); |
There was a problem hiding this comment.
isn't Object.create(Object.getPrototypeOf({})) same as {}?
There was a problem hiding this comment.
probably, but with the object literal we ran into type issues again
|
Got released with |
* master: refactor(editor): Migrate part of the vuex store to pinia (#4484) fix(editor): fix themes the monaco editor (no-changelog) (#4521) refactor(editor): restrict mapping discoverability tooltip showing only on string input (#4496) 📚 Remove entries from Changelog 📚 Update CHANGELOG.md and main package.json to 0.201.0 🔖 Release [email protected] ⬆️ Set [email protected], [email protected], [email protected] and [email protected] on n8n 🔖 Release [email protected] ⬆️ Set [email protected] and [email protected] on n8n-editor-ui 🔖 Release [email protected] 🔖 Release [email protected] ⬆️ Set [email protected] and [email protected] on n8n-nodes-base 🔖 Release [email protected] ⬆️ Set [email protected] and [email protected] on n8n-node-dev 🔖 Release [email protected] ⬆️ Set [email protected] on n8n-core 🔖 Release [email protected] fix(core): make `deepCopy` backward compatible (#4505) fix(editor): workflow settings link font size change (no-changelog) (#4511) perf(editor): improve array intersection utility function (#4503) # Conflicts: # packages/editor-ui/src/App.vue # packages/editor-ui/src/components/ActivationModal.vue # packages/editor-ui/src/components/NDVDraggablePanels.vue # packages/editor-ui/src/components/NodeSettings.vue # packages/editor-ui/src/components/Sticky.vue # packages/editor-ui/src/components/mixins/workflowHelpers.ts # packages/editor-ui/src/constants.ts # packages/editor-ui/src/plugins/i18n/index.ts # packages/editor-ui/src/views/ChangePasswordView.vue # packages/editor-ui/src/views/NodeView.vue # packages/editor-ui/src/views/SignupView.vue
JSON.parse(JSON.stringify())uses.toJSONwhen available. so shoulddeepCopy