bito-code-review[bot] commented on code in PR #41511:
URL: https://github.com/apache/superset/pull/41511#discussion_r3493500050
##########
superset-frontend/plugins/plugin-chart-cartodiagram/src/components/OlChartMap.tsx:
##########
@@ -24,7 +24,7 @@ import { View } from 'ol';
import BaseEvent from 'ol/events/Event';
import { unByKey } from 'ol/Observable';
import { toLonLat } from 'ol/proj';
-import { debounce } from 'lodash';
+import { debounce } from 'lodash-es';
Review Comment:
<!-- Bito Reply -->
The reviewer's suggestion to remove the import is incorrect because the
imported `debounce` is indeed utilized in the zoom effect at line 305. The
local `debounce` function defined earlier in the file does not replace the need
for the imported version in that specific context.
**superset-frontend/plugins/plugin-chart-cartodiagram/src/components/OlChartMap.tsx**
```
-import { debounce } from 'lodash';
+import { debounce } from 'lodash-es';
```
##########
superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx:
##########
@@ -104,9 +104,9 @@ jest.mock(
}),
);
-jest.mock('lodash/debounce', () => ({
- __esModule: true,
- default: (fuc: Function) => fuc,
+jest.mock('lodash', () => ({
Review Comment:
<!-- Bito Reply -->
The suggestion is valid. While the `jest.config.js` alias ensures that
imports of `lodash-es` resolve to the CJS build of `lodash`, mocking the module
by its exact import specifier (`lodash-es`) is the standard practice to ensure
the mock is correctly applied to the component's specific import statement.
**superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx**
```
jest.mock('lodash-es', () => ({
__esModule: true,
default: (fuc: Function) => fuc,
}));
```
##########
superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx:
##########
@@ -37,11 +37,14 @@ import { DrillBySubmenu, DrillBySubmenuProps } from
'./DrillBySubmenu';
const { form_data: defaultFormData } = chartQueries[sliceId];
-jest.mock('lodash/debounce', () => (fn: Function & { debounce: Function }) => {
- // eslint-disable-next-line no-param-reassign
- fn.debounce = jest.fn();
- return fn;
-});
+jest.mock('lodash', () => ({
+ ...jest.requireActual('lodash'),
+ debounce: (fn: Function & { debounce: Function }) => {
+ // eslint-disable-next-line no-param-reassign
+ fn.debounce = jest.fn();
+ return fn;
+ },
+}));
Review Comment:
<!-- Bito Reply -->
The suggestion to mock 'lodash' instead of 'lodash/debounce' is appropriate
in this context. Since the project configuration maps 'lodash-es' to the
CommonJS build of 'lodash', mocking the 'lodash' package correctly intercepts
the import, ensuring the mock is applied as intended.
**superset-frontend/src/components/Chart/DrillBy/DrillBySubmenu.test.tsx**
```
jest.mock('lodash', () => ({
...jest.requireActual('lodash'),
debounce: (fn: Function & { debounce: Function }) => {
// eslint-disable-next-line no-param-reassign
fn.debounce = jest.fn();
return fn;
},
}));
```
##########
superset-frontend/src/explore/components/DataTableControl/FilterInput.test.tsx:
##########
@@ -19,9 +19,9 @@
import { render, screen, userEvent } from 'spec/helpers/testing-library';
import { FilterInput } from '.';
-jest.mock('lodash/debounce', () => ({
- __esModule: true,
- default: (fuc: Function) => fuc,
+jest.mock('lodash', () => ({
Review Comment:
<!-- Bito Reply -->
The configuration in `jest.config.js` correctly maps `lodash-es` to the
CommonJS build of `lodash`, which allows the mock to intercept the import as
intended. Since the mock is correctly intercepting the module, the test
behavior is consistent with the intended synchronous passthrough.
**superset-frontend/src/explore/components/DataTableControl/FilterInput.test.tsx**
```
jest.mock('lodash', () => ({
```
--
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]
