bito-code-review[bot] commented on PR #41390:
URL: https://github.com/apache/superset/pull/41390#issuecomment-4902580159
<!-- Bito Reply -->
The flagged issue is correct. Using a fixed string key for
`Object.defineProperty` can indeed cause collisions if the user's dataset
already contains a field named `__basicColorFormatters__`. To resolve this, you
should use a `Symbol` as the key, which is guaranteed to be unique and will not
collide with any user-defined string keys.
Here is the corrected implementation for
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts`:
```typescript
// Define a unique symbol for the formatter key
export const BASIC_COLOR_FORMATTERS_ROW_KEY = Symbol('basicColorFormatters');
// ... inside transformProps ...
if (basicColorFormatters) {
passedData.forEach((row, index) => {
Object.defineProperty(row, BASIC_COLOR_FORMATTERS_ROW_KEY, {
value: basicColorFormatters[index],
enumerable: false,
configurable: true,
writable: true,
});
});
}
```
I have checked the PR comments, but there are no other comments available in
the provided context to address. If you have other comments you would like me
to review, please provide them.
**superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts**
```
if (basicColorFormatters) {
passedData.forEach((row, index) => {
Object.defineProperty(row, BASIC_COLOR_FORMATTERS_ROW_KEY, {
value: basicColorFormatters[index],
enumerable: false,
configurable: true,
writable: true,
});
});
}
```
--
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]
