Skip to content

Conversation

@shantanugupta2004
Copy link
Contributor

Fixes #15154
Addressed a React warning that occurred on the "Settings > Domains" page: Warning: Cannot update a component (Batcher) while rendering a different component (SettingsPublicDomainsListCard)

The warning was caused by a state update being called directly within the render function of the SettingsPublicDomainsListCard component. This PR fixes the issue by moving the state update into a useEffect hook. This ensures that the state is updated only after the component has rendered, which is the correct way to handle side effects in React.

Changes: Wrapped the setSelectedPublicDomain(undefined) call in a useEffect hook in SettingsPublicDomainsListCard.tsx.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR fixes a React warning that occurred when accessing the "Settings > Domains" page by properly handling side effects in the SettingsPublicDomainsListCard component. The warning "Cannot update a component (Batcher) while rendering a different component" was caused by a state update (setSelectedPublicDomain(undefined)) being called directly within the render function when publicDomains.length === 0.

The fix moves this state update into a useEffect hook with proper dependencies (publicDomains and setSelectedPublicDomain), ensuring the side effect runs after the component renders rather than during the render phase. This change follows React best practices and maintains the same functional behavior while eliminating the console warning. The component is part of the settings domain management system, allowing users to configure public domains for their workspace.

Changed Files
Filename Score Overview
packages/twenty-front/src/modules/settings/domains/components/SettingsPublicDomainsListCard.tsx 5/5 Moved state update from render function to useEffect hook to fix React warning

Confidence score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects a simple, well-implemented fix that follows React best practices and addresses a specific console warning without changing functionality
  • No files require special attention

Sequence Diagram

sequenceDiagram
    participant User
    participant SettingsPublicDomainsListCard
    participant useFindManyPublicDomainsQuery
    participant useEffect
    participant selectedPublicDomainState
    participant SettingsListCard
    participant Navigator

    User->>SettingsPublicDomainsListCard: "Opens Settings > Domains page"
    SettingsPublicDomainsListCard->>useFindManyPublicDomainsQuery: "Query public domains"
    useFindManyPublicDomainsQuery-->>SettingsPublicDomainsListCard: "Returns data and loading state"
    
    alt Data is loading or null
        SettingsPublicDomainsListCard-->>User: "Render nothing (null)"
    else No public domains exist
        SettingsPublicDomainsListCard->>useEffect: "Trigger effect with empty domains array"
        useEffect->>selectedPublicDomainState: "setSelectedPublicDomain(undefined)"
        SettingsPublicDomainsListCard-->>User: "Render 'Add Public Domain' card"
    else Public domains exist
        SettingsPublicDomainsListCard->>SettingsListCard: "Render list with domains"
        SettingsListCard-->>User: "Display domains list"
        
        alt User clicks on domain row
            User->>SettingsListCard: "Click domain row"
            SettingsListCard->>selectedPublicDomainState: "setSelectedPublicDomain(publicDomain)"
            SettingsListCard->>Navigator: "navigate(SettingsPath.PublicDomain)"
            Navigator-->>User: "Navigate to domain details page"
        else User clicks footer button
            User->>SettingsListCard: "Click 'Add Public Domain' footer button"
            SettingsListCard->>selectedPublicDomainState: "setSelectedPublicDomain(undefined)"
            SettingsListCard->>Navigator: "navigate(SettingsPath.PublicDomain)"
            Navigator-->>User: "Navigate to add domain page"
        end
    end
Loading

Context used:

  • Context from dashboard - Use useEffect in EffectComponent or hooks only. Move effects to dedicated components when necess... (source)

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link
Contributor

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:48772

This environment will automatically shut down when the PR is closed or after 5 hours.

@FelixMalfait
Copy link
Member

Thanks for this! But I think we want to solve it differently, introducing a useEffect here doesn't seem like a clean fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error in devtools while opening Domains settings

4 participants