-
Notifications
You must be signed in to change notification settings - Fork 0
chore: update .gitignore and improve error handling in workflow compo… #54
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ node_modules | |
| .pnp | ||
| .pnp.js | ||
| /.history | ||
|
|
||
| tsconfig.tsbuildinfo/ | ||
| # Local env files | ||
| .env | ||
| .env.local | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ import { getNodeConfig } from "@/app/lib/nodeConfigs"; | |||||
| import { useState } from "react"; | ||||||
| import { HOOKS_URL } from "@repo/common/zod"; | ||||||
| import { userAction } from "@/store/slices/userSlice"; | ||||||
|
||||||
| import { userAction } from "@/store/slices/userSlice"; |
Copilot
AI
Jan 14, 2026
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.
Inconsistent spacing around the comma in the console.log statement. There should be a space after the comma for standard formatting consistency.
| console.log("we are getting this userId from ConfigModal" , userId) | |
| console.log("we are getting this userId from ConfigModal", userId); |
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.
Critical: Hook called after conditional return violates Rules of Hooks.
useAppSelector on line 24 is called after the early return on line 23. React hooks must be called unconditionally and in the same order on every render. This will cause runtime errors or unpredictable behavior.
Move the hook call before the early return, and consider removing the unused userAction import on line 6.
Proposed fix
export default function ConfigModal({
isOpen,
selectedNode,
onClose,
onSave,
}: ConfigModalProps) {
const [loading, setLoading] = useState(false);
+ const userId = useAppSelector((state) => state.user.userId) as string;
if (!isOpen || !selectedNode) return null;
- const userId = useAppSelector((state) => state.user.userId) as unknown as string;
console.log("we are getting this userId from ConfigModal" , userId)Also remove the unused import:
-import { userAction } from "@/store/slices/userSlice";🧰 Tools
🪛 Biome (2.1.2)
[error] 24-24: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In `@apps/web/app/workflows/`[id]/components/ConfigModal.tsx around lines 23 - 25,
The hook call useAppSelector must be moved before the conditional early return
so hooks run unconditionally: call const userId = useAppSelector(...) (and any
other hooks) at the top of the ConfigModal component (before checking isOpen or
selectedNode) and then keep the if (!isOpen || !selectedNode) return null;
afterwards; also remove the unused userAction import referenced on line 6 to
avoid dead code.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -58,33 +58,36 @@ export default function WorkflowCanvas() { | |||||
|
|
||||||
| const handleNodesChange = (changes: NodeChange[]) => { | ||||||
| onNodesChange(changes); // Update UI first | ||||||
|
|
||||||
| changes.forEach((change) => { | ||||||
| if (change.type === "position" && change.position) { | ||||||
| // Find the node in local state to check its type | ||||||
| const changedNode = nodes.find((n) => n.id === change.id); | ||||||
|
|
||||||
| if (changedNode?.data?.nodeType === "trigger") { | ||||||
| // If it's a trigger node, update via trigger API | ||||||
| api.triggers.update({ | ||||||
| TriggerId: change.id, | ||||||
| Config: { | ||||||
| ...(typeof changedNode.data.config === "object" && | ||||||
| changedNode.data.config !== null | ||||||
| ? changedNode.data.config | ||||||
| : {}), | ||||||
| try { | ||||||
| changes.forEach((change) => { | ||||||
| if (change.type === "position" && change.position) { | ||||||
| // Find the node in local state to check its type | ||||||
| const changedNode = nodes.find((n) => n.id === change.id); | ||||||
|
|
||||||
| if (changedNode?.data?.nodeType === "trigger") { | ||||||
| // If it's a trigger node, update via trigger API | ||||||
| api.triggers.update({ | ||||||
| TriggerId: change.id, | ||||||
| Config: { | ||||||
| ...(typeof changedNode.data.config === "object" && | ||||||
| changedNode.data.config !== null | ||||||
| ? changedNode.data.config | ||||||
| : {}), | ||||||
| position: change.position, | ||||||
| }, | ||||||
| }); | ||||||
| } else { | ||||||
| // Otherwise, update in node table | ||||||
| api.nodes.update({ | ||||||
| NodeId: change.id, | ||||||
| position: change.position, | ||||||
| }, | ||||||
| }); | ||||||
| } else { | ||||||
| // Otherwise, update in node table | ||||||
| api.nodes.update({ | ||||||
| NodeId: change.id, | ||||||
| position: change.position, | ||||||
| }); | ||||||
| }); | ||||||
|
Comment on lines
+69
to
+84
|
||||||
| } | ||||||
| } | ||||||
| } | ||||||
| }); | ||||||
| }); | ||||||
| } catch (error: any) { | ||||||
| setError(error); | ||||||
|
||||||
| setError(error); | |
| setError(error instanceof Error ? error.message : String(error)); |
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.
Async API calls not awaited—try/catch won't catch errors.
The api.triggers.update and api.nodes.update calls return Promises but are not awaited. The synchronous try/catch block won't catch any errors from these async operations. Additionally, forEach doesn't handle async callbacks properly.
Proposed fix using for...of with await
const handleNodesChange = (changes: NodeChange[]) => {
onNodesChange(changes); // Update UI first
- try {
- changes.forEach((change) => {
+ changes.forEach(async (change) => {
+ try {
if (change.type === "position" && change.position) {
// Find the node in local state to check its type
const changedNode = nodes.find((n) => n.id === change.id);
if (changedNode?.data?.nodeType === "trigger") {
// If it's a trigger node, update via trigger API
- api.triggers.update({
+ await api.triggers.update({
TriggerId: change.id,
Config: {
...(typeof changedNode.data.config === "object" &&
changedNode.data.config !== null
? changedNode.data.config
: {}),
position: change.position,
},
});
} else {
// Otherwise, update in node table
- api.nodes.update({
+ await api.nodes.update({
NodeId: change.id,
position: change.position,
});
}
}
- });
- } catch (error: any) {
- setError(error);
- }
+ } catch (error: any) {
+ setError(error.message ?? "Failed to update node position");
+ }
+ });
};🤖 Prompt for AI Agents
In `@apps/web/app/workflows/`[id]/page.tsx around lines 61 - 90, The loop over
changes uses forEach and calls async functions api.triggers.update and
api.nodes.update without awaiting them, so the surrounding try/catch cannot
catch rejections; replace the forEach with an async-aware loop (e.g., for...of)
or collect promises and await Promise.all, and ensure each call to
api.triggers.update or api.nodes.update is awaited so errors propagate to the
try/catch and setError(error) will be invoked; locate this logic around the
variables changes, nodes, changedNode and update the block that currently calls
api.triggers.update(...) and api.nodes.update(...) to await the returned
promises.
Copilot
AI
Jan 14, 2026
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.
Inconsistent spacing before the closing parenthesis. There should be a space before the closing brace for consistency with line 435 and standard formatting conventions.
| await api.triggers.update({ TriggerId: nodeId, Config: config}); | |
| await api.triggers.update({ TriggerId: nodeId, Config: config }); |
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.
Incorrect pattern:
tsconfig.tsbuildinfois a file, not a directory.The trailing slash causes git to only match directories named
tsconfig.tsbuildinfo/. TypeScript build info files are files, not directories. Consider using a glob pattern to ignore all such files and consolidate the duplicate entries below (lines 43, 50, 51, 52).Suggested fix
If you use the glob pattern, you can also remove the redundant specific entries at lines 43, 50, 51, and 52.
📝 Committable suggestion
🤖 Prompt for AI Agents