Skip to content
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

[SIP-139] Proposal for Ant Design 5.x Upgrade #29268

Closed
geido opened this issue Jun 17, 2024 · 5 comments
Closed

[SIP-139] Proposal for Ant Design 5.x Upgrade #29268

geido opened this issue Jun 17, 2024 · 5 comments
Labels
change:backend Requires changing the backend change:frontend Requires changing the frontend sip Superset Improvement Proposal

Comments

@geido
Copy link
Member

geido commented Jun 17, 2024

Motivation

Superset currently utilizes Ant Design version 4, which has proven to be a significant blocker for major initiatives such as theming (see blog and SIP for the state of this effort) and accessibility improvements. The limitations and difficulties associated with Ant Design 4.x have hindered progress in achieving these goals effectively.

Goals

  • Upgrade Ant Design to the latest version.
  • Map the Superset Theme with the Ant Design theme.
  • Upgrade all Superset components to use the latest Ant Design version.

Non-Goals

Incremental Component Upgrade

The proposed solution is to upgrade each component in isolation, allowing for incremental upgrades and community participation. The immediate consequence of this approach is the necessity to support two versions of Ant Design—the current Superset version and the target version—until all components have been upgraded, and the legacy AntD version is finally removed. This situation requires the establishment of clear standards and methodologies to minimize the duration of maintaining both versions.

A proof of concept (POC) has been created, upgrading the Badge component to Ant Design 5 in isolation (#29124). This approach minimizes coordination complexity and enables community contributions while maintaining clear standards.

The POC shows the viability of this approach and also details eventual limitations and edge cases.

To ensure the transition period is as short as possible, we need a structured and efficient process:

  • Kanban Board Setup:

    • Creating a Kanban board with all components listed for the upgrade provides a clear visual overview of the progress. This helps in tracking and prioritizing the work efficiently.
  • Target Superset Release Version for Completion:

    • Setting a clear target establishes a deadline that motivates the team and stakeholders to focus on the task. Given the complexity of the work involved, Superset version 6 appears to be the most likely target.

    Note: This means that if we start the migration today, there may be AntD v4 and v5 components in the codebase at the time Superset v5 is released.

  • Prioritize Simpler Components First:

    • Upgrading simpler components initially helps in building momentum and allows the team to gain valuable insights and experience, which can be applied to more complex components later.

Detailed approach

We have identified several areas that have posed obstacles to upgrading Ant Design and have increased maintenance costs. These include:

• Custom styles
• Modifications to standard behaviors
• Insufficient Storybook and test coverage

Custom styles

Custom styles are frequently applied to tailor components to our design needs, significantly complicating the upgrade process. It is advisable to use Ant Design’s built-in theme capabilities whenever possible and only resort to custom styles after carefully considering the maintenance costs.

As we upgrade Ant Design to version 5, it is critical to reassess these choices. Here is the proposed approach:

  • Strip All Custom Styles:
    • Removing all custom styles initially ensures that we start with a clean slate. This allows us to see how the component looks with the default Ant Design theme and make necessary adjustments.
  • Integrate Ant Design Theme Correctly:
    • Proper integration of the Ant Design theme ensures consistency across components. This step is crucial for maintaining a unified look and feel.
  • Showcase States in Storybook:
    • Using Storybook to display possible states of the component helps in identifying discrepancies and ensures comprehensive testing.
  • Reapply Essential Custom Styles:
    • Only reapply custom styles that are absolutely necessary and cannot be achieved through the Ant Design theme. This minimizes maintenance overhead and potential issues with future updates. Targeting individual class names or specific DOM structures is highly discouraged as these tend to change between versions.

Modifications to standard behaviors

Heavily leveraging CSS styles in our custom components to override standard behaviors is common practice in Superset. However, this approach incurs high maintenance costs due to the challenges of upgrading to newer versions of Ant Design.

It is advisable to consider the standard, out-of-the-box behavior of components sufficient and only implement significant behavior changes if they are absolutely necessary for product requirements using the available props.

Any such changes should be thoroughly documented, showcased in Storybook, and rigorously tested.

Insufficient Storybook and test coverage

Automation and rigorous testing are essential for maintaining quality and preventing regressions. Therefore, as we proceed with upgrading Ant Design components in isolation, it is crucial to include RTL tests, Storybook, and visual regression tests as fundamental components of the definition of done for each PR.

  • Utilize Storybook:

    Storybook allows developers to visualize components in isolation and interact with different states. This is essential for ensuring the component behaves correctly in most scenarios.

  • Consider Applitools for Visual Regression Testing:

    Applitools can automate visual testing and detect UI inconsistencies that might be missed in manual testing. While it is impractical to cover all possible interactions for each component, ensuring that Storybook presents the most relevant states will help identify visual regressions when Applitools is run against our Storybook. At a minimum, the component should appear in Storybook (e.g. tooltips/popovers should be displayed somewhere in an “active” state).

  • RTL:

    RTL (React Testing Library) tests will help ensure that components behave correctly and consistently after the upgrade by focusing on user interactions and functional integrity, thereby reducing the risk of regressions. At a minimum, we want to have an RTL test that makes sure the component renders successfully. Anything beyond that is a “nice to have” and should be tracked on a separate Kanban board or task list.

Proposed Workflow Approach

The following workflow should ensure a structured and efficient upgrade process:

  1. Component Usage Check:
    • Identify all instances of the component across the application to understand its current usage and dependencies.
  2. Compare Current and New Versions:
    • Use Storybook to compare the current component state with the upgraded version, highlighting any differences.
  3. Remove Unused Props and Styles:
    • Clean up the component by removing any props and styles that are no longer necessary, incompatible with the new version, or unused.
  4. Adjust Ant Design Theme:
    • Fine-tune the Ant Design theme to match the previous look and feel as closely as possible, ensuring a seamless user experience.
  5. Minimal Custom Style Application:
    • Apply custom styles only when absolutely necessary and document these changes to facilitate future maintenance.
  6. Design Confrontation:
    • Regularly consult with the design team to ensure that customizations are necessary and maintainable, reducing long-term technical debt.
  7. Documentation and Testing:
    • Update or create Storybook stories and test cases for each upgraded component to ensure comprehensive coverage and easy reference for future developers.

Rejected Alternative Solution: Feature Branch Approach

The feature branch approach was considered but ultimately rejected due to its complexity and coordination requirements. While it allows for testing changes without risking the master branch, the need for extensive coordination and potential for prolonged support of multiple Ant Design versions outweighs its benefits. The incremental component upgrade approach, by contrast, is more manageable and enables community contributions with clear standards.

Conclusion

The proposed incremental approach, utilizing an isolated component upgrade strategy, enables a controlled and manageable upgrade path to the latest version of Ant Design. This approach minimizes coordination complexity, leverages community contributions, and unblocks essential theming and accessibility improvements, ensuring a more robust and user-friendly Superset application.

Kanban Board

A list of all Ant Design components and where they are used can be found here: #29293

@geido geido added the sip Superset Improvement Proposal label Jun 17, 2024
@dosubot dosubot bot added change:backend Requires changing the backend change:frontend Requires changing the frontend labels Jun 17, 2024
@geido geido changed the title [SIP] Proposal for Ant Design 5.x Upgrade [SIP-139] Proposal for Ant Design 5.x Upgrade Jun 17, 2024
@michael-s-molina
Copy link
Member

Thanks for the detailed SIP @geido and for leading the efforts to upgrade Ant Design.

As we upgrade Ant Design to version 5, it is critical to reassess these choices. Here is the proposed approach:

Strip All Custom Styles:
Integrate Ant Design Theme Correctly:
Showcase States in Storybook:
Reapply Essential Custom Styles:

This is great! Get ready @kasiazjc! 😄

Modifications to standard behaviors
Heavily leveraging component props to alter standard behaviors is common practice in Superset. However, this approach incurs high maintenance costs due to the challenges of upgrading to newer versions of Ant Design.

When reading this, I was a little bit confused. Altering standard behaviors using Ant Design properties is ok as they are part of the API. I believe you meant that we pass properties to our custom components which override the Ant Design component using CSS styles instead of properties. If that's what you meant, then we can improve this paragraph to make it more clear.

Proposed Workflow Approach

One suggestion I have is to create the PRs that upgrade a component as an atomic unit of work that can be reverted easily. This will allow us to require @kasiazjc's approval in each of these PRs, before merging them. We can use labels to block the merge button.

A list of all Ant Design components and where they are used.

Is this list complete? I don't see Ant Design's Select component.

@geido
Copy link
Member Author

geido commented Jun 17, 2024

Thanks @michael-s-molina! Updated.

@rusackas
Copy link
Member

If we ever get that tech debt dashboard merged, this'll be trivial to track with a new linting rule.

@geido
Copy link
Member Author

geido commented Jun 18, 2024

To improve readability I moved the list to a dedicated discussion #29293

@rusackas
Copy link
Member

Hooray, it passed! Closing and moving it on the kanban...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:backend Requires changing the backend change:frontend Requires changing the frontend sip Superset Improvement Proposal
Projects
Development

No branches or pull requests

3 participants