codeant-ai-for-open-source[bot] commented on PR #34955: URL: https://github.com/apache/superset/pull/34955#issuecomment-3639336435
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/34955/files#diff-b7aa140450d89dd54ea6fde005a508e0ef11a6d8885d4faa0d032264000d1d97R21-R36'><strong>Possible Bug</strong></a><br>The file imports the dayjs UTC plugin as a side-effect ("import 'dayjs/plugin/utc'") but no longer calls `dayjs.extend(utc)` on the `extendedDayjs` instance. The code calls `dayjs.utc(...)` when parsing metric values; if the plugin is not registered on the `dayjs` instance used here, `dayjs.utc` may be undefined and cause a runtime error. Ensure the UTC plugin is explicitly extended on the `extendedDayjs` instance used in this module.<br> - [ ] <a href='https://github.com/apache/superset/pull/34955/files#diff-b7aa140450d89dd54ea6fde005a508e0ef11a6d8885d4faa0d032264000d1d97R20-R21'><strong>Side-effect import reliability</strong></a><br>Relying on a side-effect import to register a plugin is brittle: importing the plugin without an explicit `extend` call assumes the plugin registration happens elsewhere or that importing the module mutates the exact `dayjs` instance used here. This can break in different bundler/module resolution setups. Prefer explicit registration or a safe runtime fallback.<br> - [ ] <a href='https://github.com/apache/superset/pull/34955/files#diff-5ddf475da8ebdd99944c8e90c1091af32013c1435cef0185b13e96527304b224R19-R21'><strong>Plugin ordering</strong></a><br>The file now imports the dayjs UTC plugin as a side-effect. Side-effect imports and dayjs plugin registration are order-sensitive: the plugin must be loaded/registered before the dayjs instance that needs it is created/used. Verify that the plugin is actually registered on the same dayjs instance used by `extendedDayjs` (or that `extendedDayjs` is created after the plugin import). If not, parsing using `dayjs.utc(...)` may not behave as expected.<br> - [ ] <a href='https://github.com/apache/superset/pull/34955/files#diff-95cd3ec455623897fcad6e1feffeb581875e63f1f56b3331b73d42866cb98f8cR19-R20'><strong>Possible Bug</strong></a><br>The code calls `dayjs.utc(...)` but the PR only adds a side-effect import of the UTC plugin (`import 'dayjs/plugin/utc'`). If the plugin is not explicitly extended on the same `dayjs` instance used in this file (the `extendedDayjs` imported from `@superset-ui/core/utils/dates`), `dayjs.utc` may be undefined at runtime. Verify that the plugin is actually applied to the `dayjs` instance used here.<br> - [ ] <a href='https://github.com/apache/superset/pull/34955/files#diff-95cd3ec455623897fcad6e1feffeb581875e63f1f56b3331b73d42866cb98f8cR19-R20'><strong>Code Smell</strong></a><br>The new import is a side-effect import. This can be flagged by linters and makes it unclear which dayjs instance is extended. Prefer explicit import of the plugin and call `dayjs.extend(utc)` on the specific `dayjs` instance to make the extension explicit and avoid surprising behavior.<br> - [ ] <a href='https://github.com/apache/superset/pull/34955/files#diff-95cd3ec455623897fcad6e1feffeb581875e63f1f56b3331b73d42866cb98f8cR19-R20'><strong>Type augmentation mismatch</strong></a><br>The comment indicates "Type augmentation for dayjs plugins", but merely importing the plugin file may not produce the intended TypeScript augmentation for the `extendedDayjs` type. Confirm the plugin's types are available to the `dayjs` instance exported by `@superset-ui/core/utils/dates` or add the appropriate import/extend pattern so types and runtime behavior align.<br> </td></tr> </table> -- 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]
