chore(build): reduce Lodash usage in `superset-frontend` #32341

hainenber · 2025-02-21T09:21:25Z

chore(build): reduce Lodash usage in superset-frontend

SUMMARY

Reduce some Lodash usage for cases modern JS can handle by its own.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Fix Detected
	Memoization Missing for Config Builder ▹ view

Files scanned

File Path	Reviewed
superset-websocket/eslint.config.js	✅
superset-frontend/plugins/plugin-chart-echarts/src/utils/eventHandlers.ts	✅
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Screengrid/Screengrid.tsx	✅
superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts	✅
superset-frontend/src/dashboard/components/menu/DownloadMenuItems/DownloadScreenshot.tsx	✅
superset-frontend/src/features/home/ChartTable.tsx	✅
superset-frontend/src/features/home/DashboardTable.tsx	✅
superset-websocket/src/config.ts	✅
superset-frontend/src/components/Select/AsyncSelect.tsx	✅
superset-frontend/src/components/Select/Select.tsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

Chat with Korbit on issues we post by tagging @korbit-ai in your reply.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

Check out our docs on how you can make Korbit work best for you and your team.

Customize Korbit for your organization through the Korbit Console.

Current Korbit Configuration

General Settings

Setting Value

Review Schedule Automatic excluding drafts

Max Issue Count 10

Automatic PR Descriptions ❌

Issue Categories

Category Enabled

Documentation ✅

Logging ✅

Error Handling ✅

Readability ✅

Design ✅

Performance ✅

Security ✅

Functionality ✅

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ [email protected]

Note

Korbit Pro is free for open source projects 🎉

Looking to add Korbit to your team? Get started with a free 2 week trial here

superset-websocket/src/config.ts

kgabryje · 2025-02-21T10:13:28Z

superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts

-  const anchor = useSelector(
-    (state: RootState) =>
-      last(state.dashboardState.directPathToChild) || undefined,
+  const anchor = useSelector((state: RootState) =>


I think that last looks much more readable, especially when we have such long array names. Maybe we should either keep lodash or implement our own last util function?

Yes can, I'll put in our own last impl.

I agree with @kgabryje but don't think we should implement our own function. We need to keep in mind that any function that we implement needs to be maintained and in general it's best to depend on libraries that are greatly supported like Lodash.

michael-s-molina

Thank you for the PR @hainenber. I think some of the changes are beneficial like filter changes but others decrease readability and increase our maintenance cost. I left comments on the ones that I think we should preserve. As a general guidance, I think it's always better to rely on greatly supported libraries like Lodash instead of rewriting code which requires testing and support.

michael-s-molina · 2025-02-21T11:23:27Z

superset-frontend/src/components/Select/AsyncSelect.tsx

@@ -574,7 +574,9 @@ const AsyncSelect = forwardRef(
        }
      } else {
        const token = tokenSeparators.find(token => pastedText.includes(token));
-        const array = token ? uniq(pastedText.split(token)) : [pastedText];
+        const array = token


Even though we can implement this using simple Javascript, uniq still offers semantic meaning that helps with readability.

michael-s-molina · 2025-02-21T11:25:11Z

superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts

-  const anchor = useSelector(
-    (state: RootState) =>
-      last(state.dashboardState.directPathToChild) || undefined,
+  const anchor = useSelector((state: RootState) =>


I agree with @kgabryje but don't think we should implement our own function. We need to keep in mind that any function that we implement needs to be maintained and in general it's best to depend on libraries that are greatly supported like Lodash.

michael-s-molina · 2025-02-21T11:25:47Z

superset-websocket/src/config.ts

@@ -142,6 +140,50 @@ function applyEnvOverrides(config: ConfigType): ConfigType {
 }

 export function buildConfig(): ConfigType {
-  const config = _merge(defaultConfig(), configFromFile());


Previous version is way more readable and less error prone.

michael-s-molina · 2025-02-21T11:28:37Z

superset-frontend/src/features/home/ChartTable.tsx

@@ -76,7 +75,7 @@ function ChartTable({
    TableTab.Other,
  );

-  const filteredOtherTabData = filter(otherTabData, obj => 'viz_type' in obj);
+  const filteredOtherTabData = otherTabData?.filter(obj => 'viz_type' in obj);


hainenber · 2025-02-21T11:38:16Z

Thanks all for the feedbacks. Looking at this PR retrospectively, I think I optimized too much at the expense of other factors like readability and module maturity. I'll retain the .filter and .noop replacements and discard others.

Edit: Apologies for a force push 🙏

…rn JS alternatives

Signed-off-by: hainenber <[email protected]>

michael-s-molina

Thank you for addressing the comments @hainenber. You have been one of the best additions to our committers roster. Keep up the excellent work 👏🏼

hainenber requested review from michael-s-molina, geido and kgabryje as code owners February 21, 2025 09:21

pull-request-size bot added the size/M label Feb 21, 2025

github-actions bot added plugins dependencies:npm labels Feb 21, 2025

dosubot bot added the change:frontend Requires changing the frontend label Feb 21, 2025

korbit-ai bot reviewed Feb 21, 2025

View reviewed changes

superset-websocket/src/config.ts Outdated Show resolved Hide resolved

pull-request-size bot added size/L and removed size/M labels Feb 21, 2025

kgabryje reviewed Feb 21, 2025

View reviewed changes

michael-s-molina requested changes Feb 21, 2025

View reviewed changes

chore(frontend): replace Lodash usage .filter and .noop with mode…

03d7877

…rn JS alternatives

hainenber force-pushed the chore/reduce-lodash-dependency branch from 882f03e to 03d7877 Compare February 21, 2025 11:42

pull-request-size bot added size/S and removed size/L labels Feb 21, 2025

github-actions bot removed the dependencies:npm label Feb 21, 2025

hainenber changed the title ~~chore(build): remove Lodash runtime dependency for superset-websocket and reduce Lodash usage in superset-frontend~~ chore(build): reduce Lodash usage in superset-frontend Feb 21, 2025

fix(ci): re-add incidental Lodash refactor

68f9dc6

Signed-off-by: hainenber <[email protected]>

michael-s-molina approved these changes Feb 21, 2025

View reviewed changes

hainenber merged commit f820f9a into apache:master Feb 21, 2025
48 checks passed

hainenber deleted the chore/reduce-lodash-dependency branch February 21, 2025 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(build): reduce Lodash usage in `superset-frontend` #32341

chore(build): reduce Lodash usage in `superset-frontend` #32341

hainenber commented Feb 21, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

kgabryje Feb 21, 2025

hainenber Feb 21, 2025

michael-s-molina Feb 21, 2025

michael-s-molina left a comment

michael-s-molina Feb 21, 2025

michael-s-molina Feb 21, 2025

michael-s-molina Feb 21, 2025

michael-s-molina Feb 21, 2025

hainenber commented Feb 21, 2025 •

edited

Loading

michael-s-molina left a comment

Setting	Value
Review Schedule	Automatic excluding drafts
Max Issue Count	10
Automatic PR Descriptions	❌

Category	Enabled
Documentation	✅
Logging	✅
Error Handling	✅
Readability	✅
Design	✅
Performance	✅
Security	✅
Functionality	✅

chore(build): reduce Lodash usage in superset-frontend #32341

chore(build): reduce Lodash usage in superset-frontend #32341

Conversation

hainenber commented Feb 21, 2025 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot left a comment • edited Loading

Choose a reason for hiding this comment

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Interacting with Korbit

Customizing Korbit

Current Korbit Configuration

Feedback and Support

Looking to add Korbit to your team? Get started with a free 2 week trial here

kgabryje Feb 21, 2025

Choose a reason for hiding this comment

hainenber Feb 21, 2025

Choose a reason for hiding this comment

michael-s-molina Feb 21, 2025

Choose a reason for hiding this comment

michael-s-molina left a comment

Choose a reason for hiding this comment

michael-s-molina Feb 21, 2025

Choose a reason for hiding this comment

michael-s-molina Feb 21, 2025

Choose a reason for hiding this comment

michael-s-molina Feb 21, 2025

Choose a reason for hiding this comment

michael-s-molina Feb 21, 2025

Choose a reason for hiding this comment

hainenber commented Feb 21, 2025 • edited Loading

michael-s-molina left a comment

Choose a reason for hiding this comment

chore(build): reduce Lodash usage in `superset-frontend` #32341

chore(build): reduce Lodash usage in `superset-frontend` #32341

hainenber commented Feb 21, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

hainenber commented Feb 21, 2025 •

edited

Loading