gabotorresruiz commented on PR #35349:
URL: https://github.com/apache/superset/pull/35349#issuecomment-3399310969
I really appreciate the effort to improve theme validation, but I’m
concerned about the long-term maintainability of this approach. Here are my
thoughts:
`Ant Design` silently ignores invalid tokens by design. It's permissive and
forward compatible. This means:
- `Token typos` --> ignored by AntD, defaults are used
- `"#invalid"` values --> ignored by AntD, defaults are used
- Unknown tokens from future versions --> continue to work without
validation updates
By introducing custom validators (`useThemeValidation.ts`,
`themeTokenValidation.ts`), we're essentially:
1. Reimplementing `Ant Design's` token system within our own validation layer
2. Adding maintenance overhead. Every `Ant Design` update would require
updating our validators
3. Risking incompatibility. Valid future tokens could be rejected by our
outdated validation
So I think we should keep it simple and only have structural validations:
- JSON syntax validation
- Empty theme blocking
- Algorithm type validation ("dark" | "light" | "system")
- Object type validation (token, components)
So the `ENHANCED_THEME_VALIDATION` feature flag, even though it’s optional,
I think that if we make an structural validation, we might not need it anymore.
To sum up, I think we should just simplify to:
- Basic structural validation (syntax, type, emptiness)
- Runtime fallback if theme loading fails
- User notifications for load failures
This gives users clear feedback while avoiding the complexity of duplicating
`Ant Design’s` validation logic.
What do you think? Happy to discuss trade-offs!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
