Fix: Agent links, agent form, and react-compile #406
Conversation
WalkthroughThis pull request removes an unused Babel dependency and updates several metadata generation functions by destructuring parameters and switching from try‑catch error handling to promise chaining. Additionally, URL generation for job agents is modified, with changes applied to both the URL helper and the link component. Minor configuration updates include expanding the TypeScript exclusion list and disabling an experimental React compiler setting. Changes
Sequence Diagram(s)sequenceDiagram
participant P as Page Component
participant GM as generateMetadata
participant API as Workspace API
P->>GM: Call generateMetadata({ params: { workspaceSlug } })
GM->>API: Fetch workspace data using workspaceSlug
alt Successful response
API-->>GM: Return workspace data
GM->>GM: Construct metadata object from data
else Error response
API-->>GM: Return error
GM->>GM: Return default metadata values
end
GM-->>P: Return metadata object
sequenceDiagram
participant Selector as JobAgentSelector Component
participant URLHelper as URLs Module
participant Router as Next.js Router
Selector->>URLHelper: Generate job agents URL using workspace slug
URLHelper-->>Selector: Return updated URL with .baseUrl() appended
Selector->>Router: Navigate to the constructed URL
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/webservice/src/components/xterm/SessionTerminal.tsx (1)
96-96:⚠️ Potential issueReference to potentially undefined variables
The
widthandheightvariables are used here but they're only defined later in the code (line 101). This could lead to undefined variable access issues.- onResize?.({ width, height, cols, rows }); + // Use terminal dimensions or provide fallback values + onResize?.({ width: term.element?.clientWidth || 0, height: term.element?.clientHeight || 0, cols, rows });
🧹 Nitpick comments (2)
apps/webservice/src/app/[workspaceSlug]/(app)/settings/workspace/members/MembersTable.tsx (2)
200-201: Consider refactoring to follow React hooks rules.The ESLint disable comment is used to suppress a valid warning about hooks rules. While this works, it's better to refactor the component to properly follow React's rules of hooks.
You could extract this logic into a custom component that calls the hook at its top level:
- cell: ({ row }) => { - const viewer = api.user.viewer.useQuery(); - const { id: currentRoleId } = row.original.role; - const { id: entityRoleId } = row.original.entityRole; - const roles = api.workspace.roles.useQuery(row.original.workspace.id); - const updateRole = api.workspace.iam.set.useMutation(); - // eslint-disable-next-line react-hooks/rules-of-hooks - const router = useRouter(); - const isCurrentUser = row.original.user.id === viewer.data?.id; + cell: ({ row }) => <RoleCell row={row} /> } + const RoleCell = ({ row }: { row: Row<Member> }) => { + const viewer = api.user.viewer.useQuery(); + const { id: currentRoleId } = row.original.role; + const { id: entityRoleId } = row.original.entityRole; + const roles = api.workspace.roles.useQuery(row.original.workspace.id); + const updateRole = api.workspace.iam.set.useMutation(); + const router = useRouter(); + const isCurrentUser = row.original.user.id === viewer.data?.id;
248-249: Consider refactoring to follow React hooks rules.Similar to the previous issue, this ESLint disable comment is suppressing a valid warning about React hooks rules.
Extract this logic into a dedicated component:
- cell: ({ row }) => { - const viewer = api.user.viewer.useQuery(); - const { id } = row.original.entityRole; - // eslint-disable-next-line react-hooks/rules-of-hooks - const router = useRouter(); - const remove = api.workspace.iam.remove.useMutation(); - if (id == null) return null; + cell: ({ row }) => <ActionsCell row={row} /> } + const ActionsCell = ({ row }: { row: Row<Member> }) => { + const viewer = api.user.viewer.useQuery(); + const { id } = row.original.entityRole; + const router = useRouter(); + const remove = api.workspace.iam.remove.useMutation(); + if (id == null) return null;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/webservice/eslint.config.js(2 hunks)apps/webservice/package.json(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(sidebar)/job-agents/page.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(sidebar)/systems/page.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/_components/reactflow/layout.ts(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/insights/page.tsx(1 hunks)apps/webservice/src/app/[workspaceSlug]/(app)/settings/workspace/members/MembersTable.tsx(2 hunks)apps/webservice/src/app/urls.ts(1 hunks)apps/webservice/src/components/form/job-agent/JobAgentSelector.tsx(1 hunks)apps/webservice/src/components/xterm/SessionTerminal.tsx(1 hunks)apps/webservice/tsconfig.json(1 hunks)tooling/eslint/package.json(2 hunks)tooling/eslint/react-compiler.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: **Note on Error Handling:** Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error...
**/*.{ts,tsx}: Note on Error Handling:
Avoid strict enforcement of try/catch blocks. Code may use early returns, Promise chains (.then().catch()), or other patterns for error handling. These are acceptable as long as they maintain clarity and predictability.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsxapps/webservice/src/app/[workspaceSlug]/(app)/_components/reactflow/layout.tsapps/webservice/src/components/form/job-agent/JobAgentSelector.tsxapps/webservice/src/components/xterm/SessionTerminal.tsxapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(sidebar)/systems/page.tsxapps/webservice/src/app/[workspaceSlug]/(app)/insights/page.tsxapps/webservice/src/app/[workspaceSlug]/(app)/settings/workspace/members/MembersTable.tsxapps/webservice/src/app/urls.tsapps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(sidebar)/job-agents/page.tsx
🧬 Code Definitions (1)
apps/webservice/src/components/form/job-agent/JobAgentSelector.tsx (1)
apps/webservice/src/app/urls.ts (1) (1)
urls(195-195)
🔇 Additional comments (17)
apps/webservice/src/app/[workspaceSlug]/(app)/_components/reactflow/layout.ts (2)
1-1: ESLint directive added to disable react-compiler ruleThis ESLint directive disables the React compiler rule for this entire file. This is consistent with the React compiler integration mentioned in the PR title.
3-3: "use no memo" directive addedThis directive instructs the React compiler to avoid automatic memoization for components and functions in this file. Since this file handles complex ReactFlow layout operations which may have intricate state dependencies, disabling automatic memoization is appropriate.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(raw)/systems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx (1)
2-2: "use no memo" directive addedThis directive disables automatic memoization by the React compiler for this file. This is appropriate for a complex component like JobAgentSection that manages form state and interactions with various agent types.
apps/webservice/src/components/xterm/SessionTerminal.tsx (2)
1-1: ESLint directive added to disable react-compiler ruleThis ESLint directive disables the React compiler rule for the entire file. This is consistent with the React compiler integration mentioned in the PR.
3-3: "use no memo" directive addedThis directive disables automatic memoization for this terminal component. This is appropriate for components handling real-time data and DOM interactions like the XTerm terminal, where automatic memoization might interfere with rendering or event handling.
apps/webservice/tsconfig.json (1)
25-25: Updated TypeScript exclude pathsAdded
.nextto the exclude list, which is a good practice for Next.js projects. This prevents TypeScript from processing generated files in the.nextdirectory, improving build performance.tooling/eslint/react-compiler.js (1)
1-14: New ESLint configuration for React CompilerThis new configuration file sets up ESLint integration for the React compiler, targeting TypeScript files. The configuration correctly imports the plugin and sets the rule to trigger errors when violated.
This aligns with the PR objectives of fixing React compiler integration. The setup allows for better automatic optimizations and potential performance improvements in React components.
apps/webservice/src/components/form/job-agent/JobAgentSelector.tsx (1)
56-56: URL generation updated correctlyThe URL generation has been properly updated to use
.baseUrl(), which aligns with the URL structure refactoring mentioned in the PR title for fixing agent links.apps/webservice/package.json (1)
105-105: React compiler plugin updated to newer beta versionThe babel-plugin-react-compiler has been updated to a newer beta version (19.0.0-beta-3229e95-20250315). This update aligns with the PR's objective to fix react-compile related issues.
tooling/eslint/package.json (2)
9-10: React compiler ESLint configuration export addedAdded a new export for the React compiler configuration file, making it available for import in other packages.
31-31: ESLint plugin for React compiler addedAdded the eslint-plugin-react-compiler as a devDependency with the same version as the babel plugin (19.0.0-beta-3229e95-20250315). This addition enables proper linting support for code using the React compiler.
apps/webservice/eslint.config.js (2)
4-4: React compiler ESLint configuration importedAdded import for the React compiler ESLint configuration from the newly available path.
15-15: React compiler ESLint rules integratedThe React compiler ESLint rules have been correctly integrated into the existing configuration array. This completes the setup needed for proper React compiler support in the webservice application.
apps/webservice/src/app/urls.ts (1)
56-56: Improved consistency in URL handling.This change refactors the
integrationsmethod to reuse the existingworkspaceSettingsIntegrationsfunction instead of custom URL building. This promotes code reuse and ensures consistent URL structures across the application.apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(sidebar)/systems/page.tsx (1)
7-24: Good refactoring of metadata generation function.The changes to
generateMetadataimprove the code by:
- Using direct parameter destructuring for cleaner access to
workspaceSlug- Implementing promise chaining instead of try-catch for more concise error handling
- Directly returning the metadata based on API call results
This approach aligns with the project's coding guidelines that allow flexible error handling patterns.
apps/webservice/src/app/[workspaceSlug]/(app)/insights/page.tsx (1)
22-40: Good refactoring of metadata generation function.This change follows the same pattern used in other files, improving the code by:
- Using direct parameter destructuring for cleaner access to
workspaceSlug- Implementing promise chaining instead of try-catch for more concise error handling
- Directly returning the metadata based on API call results
The consistent application of this pattern across files shows good attention to maintaining code style conventions throughout the codebase.
apps/webservice/src/app/[workspaceSlug]/(app)/(deploy)/(sidebar)/job-agents/page.tsx (1)
20-37: Improved code organization with cleaner promise-based error handling.The refactoring from a try/catch block to a promise chain with
.then()and.catch()aligns with the project's coding guidelines while maintaining the same functionality. The parameter destructuring makes the code more readable by directly accessing the needed properties.
...stems/[systemSlug]/(raw)/deployments/[deploymentSlug]/(sidebar)/workflow/JobAgentSection.tsx
Outdated
Show resolved
Hide resolved
apps/webservice/src/app/[workspaceSlug]/(app)/settings/workspace/members/MembersTable.tsx
Outdated
Show resolved
Hide resolved
Summary by CodeRabbit
Chores
Refactor