-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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(editor): Improve node rendering performance when opening large workflows #7904
Conversation
Passing run #3163 ↗︎
Details:
Review all test suite changes for PR #7904 ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome
can you update the checklist? have you tried adding an e2e test to open a huge workflow? I wonder if that will work.. |
Not yet, since now we have a project we can tackle adding performance tests as part of it if that sounds reasonable |
✅ All Cypress E2E specs passed |
# [1.20.0](https://github.com/n8n-io/n8n/compare/n8n@1.19.0...n8n@1.20.0) (2023-12-06) ### Bug Fixes * **AWS DynamoDB Node:** Improve error message parsing ([#7793](#7793)) ([5ba5ed8](5ba5ed8)) * **core:** Allow grace period for binary data deletion after manual execution ([#7889](#7889)) ([61d8aeb](61d8aeb)) * **core:** Consolidate ownership and sharing data on workflows and credentials ([#7920](#7920)) ([38b88b9](38b88b9)) * **core:** Fix hard deletes stopping if database query throws ([#7848](#7848)) ([46dd4d3](46dd4d3)) * **core:** Make sure mfa secret and recovery codes are not returned on login ([#7936](#7936)) ([f5502cc](f5502cc)) * **editor:** Fix deletion of last execution at execution preview ([#7883](#7883)) ([ce2d388](ce2d388)) * **editor:** Replace isInstanceOwner checks with scopes where applicable ([#7858](#7858)) ([132d691](132d691)) * **Google Sheets Node:** Fix issue with paired items not being set correctly ([#7862](#7862)) ([5207a2f](5207a2f)) * **Notion Node:** Fix broken Notion node parameters ([#7864](#7864)) ([51d1f5b](51d1f5b)), closes [#7791](#7791) ### Features * **BambooHR Node:** Add support for Only Current on company reports ([#7878](#7878)) ([4175801](4175801)) * **core:** Allow admin creation ([#7837](#7837)) ([476806e](476806e)) * **editor:** Add sections to create node panel ([#7831](#7831)) ([39fa8d2](39fa8d2)) * **editor:** Open template credential setup from collection ([#7882](#7882)) ([627ddb9](627ddb9)) * **editor:** Select credentials in template setup if theres only one ([#7879](#7879)) ([fe3417a](fe3417a)) ### Performance Improvements * **editor:** Improve node rendering performance when opening large workflows ([#7904](#7904)) ([a8049a0](a8049a0)) * **editor:** Improve performance when opening large workflows with node issues ([#7901](#7901)) ([4bd7ae2](4bd7ae2)) Co-authored-by: ivov <ivov@users.noreply.github.com>
Got released with |
Summary
In an effort to do as little processing as possible in each
Node
component, this PR passes current workflow object to it as a property instead of calling the slowgetCurrentWorkflow
store getter a few times in each node. This should substantially improve loading times for large workflows.As a benchmark, I was using a workflow from this Linear ticket and this fix brought down opening time by 20 seconds. Together with fixes from #7901, this workflow was opening in less than 10 seconds on my laptop.
Latest e2e run
How to test the change:
Issues fixed
ADO-1523
https://community.n8n.io/t/ui-very-slow-with-more-than-100-nodes/8236/14
Review / Merge checklist
(no-changelog)
otherwise. (conventions)