FrancescoCastaldi commented on code in PR #44894:
URL: https://github.com/apache/superset/pull/44894#discussion_r4262693701
##########
superset-frontend/packages/superset-ui-core/src/components/Select/utils.tsx:
##########
@@ -54,23 +58,47 @@ export function isEqual(
// When comparing the values we use the equality
// operator to automatically convert different types
// eslint-disable-next-line eqeqeq
- return actualA == actualB;
+ if (actualA == actualB) {
+ return true;
+ }
+ if (typeof actualA === 'string' && typeof actualB === 'string') {
+ return actualA.trim() === actualB.trim();
Review Comment:
I refined the equality invariant in isEqual (utils.tsx) to handle empty
strings and whitespace without unwanted collapses:
```tsx
if (
(typeof actualA === 'number' &&
typeof actualB === 'string' &&
actualB.trim() === '') ||
(typeof actualB === 'number' &&
typeof actualA === 'string' &&
actualA.trim() === '')
) {
return false;
}
if (actualA == actualB) {
return true;
}
if (typeof actualA === 'string' && typeof actualB === 'string') {
const trimmedA = actualA.trim();
const trimmedB = actualB.trim();
return trimmedA.length > 0 && trimmedA === trimmedB;
}
return false;
```
With this adjustment:
1. Empty string vs whitespace: isEqual('', ' ') evaluates to false, because
the trimmed string has length 0 and empty strings are strictly distinguished
from spaces.
2. Distinct whitespace inputs: isEqual(' ', ' ') evaluates to false.
3. Identical inputs: isEqual('', '') evaluates to true, and isEqual(' ', '
') evaluates to true.
4. Surrounding whitespace deduplication: isEqual(' Sales ', 'Sales')
evaluates to true, preserving the expected option deduplication for labels.
5. Numeric zero: isEqual(0, '') and isEqual(0, ' ') evaluate to false,
preventing loose coercion collapse. Meanwhile, isEqual(0, '0') and isEqual(1,
'1') evaluate to true as intended.
I also added comprehensive regression tests in utils.test.ts covering all
these edge cases.
Francesco Castaldi
--
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]
