-
Notifications
You must be signed in to change notification settings - Fork 54.3k
feat(editor): Add workflow documents store abstraction (no-changelog) #25031
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
base: master
Are you sure you want to change the base?
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Make workflowId parameter optional in useWorkflowState for backward compatibility - Remove computed wrapper in workflowDocuments.store.ts (Pinia auto-unwraps refs) - Add fallback to use workflows store directly when no workflowId is provided Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change useWorkflowState(workflowId?: string) to useWorkflowState(workflowId: string) - Simplify implementation to always use useWorkflowDocumentsStore - Keep fallback in injectWorkflowState for circular dependency with useWorkflowHelpers - Update NodeView.vue to compute workflowId before calling useWorkflowState - Update useCanvasOperations.ts to pass data.id - Update all test files to pass 'test-workflow-id' Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Bundle ReportChanges will increase total bundle size by 40.94MB (100.0%) ⬆️
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment has been minimized.
This comment has been minimized.
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.
1 issue found across 21 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts">
<violation number="1" location="packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts:199">
P2: Use the same workflow id as the seeded test data so the workflow state is scoped to the correct document key.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| setActivePinia(pinia); | ||
|
|
||
| workflowState = useWorkflowState(); | ||
| workflowState = useWorkflowState('test-workflow-id'); |
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.
P2: Use the same workflow id as the seeded test data so the workflow state is scoped to the correct document key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/frontend/editor-ui/src/app/composables/useCanvasOperations.test.ts, line 199:
<comment>Use the same workflow id as the seeded test data so the workflow state is scoped to the correct document key.</comment>
<file context>
@@ -196,7 +196,7 @@ describe('useCanvasOperations', () => {
setActivePinia(pinia);
- workflowState = useWorkflowState();
+ workflowState = useWorkflowState('test-workflow-id');
vi.mocked(injectWorkflowState).mockReturnValue(workflowState);
});
</file context>
| workflowState = useWorkflowState('test-workflow-id'); | |
| workflowState = useWorkflowState(workflowId); |
|
|
||
| const storesCache = new Map<string, ReturnType<typeof createWorkflowDocumentStore>>(); | ||
|
|
||
| export function useWorkflowDocumentsStore(id: string) { |
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.
is it ever removed?
Remove the manual module-level store cache in workflowDocuments.store.ts which was causing test isolation issues. The cache persisted across tests, holding stale references to Pinia store instances from previous test runs. In useWorkflowState.ts, bypass the documents store for now and use the workflows store directly. The documents store infrastructure remains for future multi-document support, using Pinia's built-in store deduplication instead of a manual cache. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
No issues found across 21 files
Summary
This pull request introduces a significant refactor to how workflow state is managed in the frontend application. The main change is that the
useWorkflowStatecomposable now requires aworkflowIdparameter and delegates workflow data access and mutation to the newuseWorkflowDocumentsStore. This change improves modularity and ensures workflow state is correctly scoped per workflow. The update also touches many test files to align with the new API and updates all internal references from the global workflow store to the workflow-specific data.Key changes include:
Core Workflow State Refactor:
useWorkflowStatenow takes aworkflowIdparameter, and internally usesuseWorkflowDocumentsStoreto manage workflow data instead of the global store. All references to workflow data inuseWorkflowState.tsare updated to use the workflow and workflow object fromuseWorkflowDocumentsStore. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]Test and Helper Updates:
useWorkflowStatein test files are updated to pass aworkflowIdargument, ensuring correct scoping in tests and composables. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]Workflow Initialization:
initializeWorkspacefunction inuseCanvasOperations.tsnow callsuseWorkflowStatewith the workflow's id, ensuring state is initialized for the correct workflow.Store Constants:
WORKFLOW_DOCUMENTSentry to theSTORESconstants, reflecting the new store for workflow documents.Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/CAT-2237/create-useworkflowdocuments-store
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)