michael-s-molina
commented
3 months ago 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
rusackas
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
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
Badgecomponent to Ant Design 5 in isolation (https://github.com/apache/superset/pull/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:
Target Superset Release Version for Completion:
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:
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:
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:
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: https://github.com/apache/superset/discussions/29293