Skip to content

chore: replace Lodash usage with native JS implementation #31907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rusackas merged 1 commit into from
Jan 21, 2025
Merged

chore: replace Lodash usage with native JS implementation #31907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
* under the License.
*/
import { JsonObject } from '@superset-ui/core';
import { isString } from 'lodash';

export const getTimeOffset = (
series: JsonObject,
Expand All @@ -36,7 +35,9 @@ export const hasTimeOffset = (
series: JsonObject,
timeCompare: string[],
): boolean =>
isString(series.name) ? !!getTimeOffset(series, timeCompare) : false;
typeof series.name === 'string'
? !!getTimeOffset(series, timeCompare)
: false;

export const getOriginalSeries = (
seriesName: string,
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/packages/superset-ui-core/src/components/SafeMarkdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { useEffect, useMemo, useState } from 'react';
import rehypeSanitize, { defaultSchema } from 'rehype-sanitize';
import remarkGfm from 'remark-gfm';
import { mergeWith, isArray } from 'lodash';
import { mergeWith } from 'lodash';
import { FeatureFlag, isFeatureEnabled } from '../utils';

interface SafeMarkdownProps {
Expand All @@ -33,7 +33,7 @@ export function getOverrideHtmlSchema(
htmlSchemaOverrides: SafeMarkdownProps['htmlSchemaOverrides'],
) {
return mergeWith(originalSchema, htmlSchemaOverrides, (objValue, srcValue) =>
isArray(objValue) ? objValue.concat(srcValue) : undefined,
Array.isArray(objValue) ? objValue.concat(srcValue) : undefined,
);
}

Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/packages/superset-ui-core/src/query/normalizeOrderBy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { isEmpty, isBoolean } from 'lodash';
import { isEmpty } from 'lodash';

import { QueryObject } from './types';

Expand All @@ -30,7 +30,7 @@ export default function normalizeOrderBy(
Array.isArray(orderbyClause) &&
orderbyClause.length === 2 &&
!isEmpty(orderbyClause[0]) &&
isBoolean(orderbyClause[1])
typeof orderbyClause[1] === 'boolean'
) {
return queryObject;
}
Expand Down
3 changes: 1 addition & 2 deletions superset-frontend/plugins/legacy-preset-chart-deckgl/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
QueryFormData,
SequentialScheme,
} from '@superset-ui/core';
import { isNumber } from 'lodash';
import { hexToRGB } from './utils/colors';

const DEFAULT_NUM_BUCKETS = 10;
Expand Down Expand Up @@ -140,7 +139,7 @@ export function getBreakPointColorScaler(
} else {
// interpolate colors linearly
const linearScaleDomain = extent(features, accessor);
if (!linearScaleDomain.some(isNumber)) {
if (!linearScaleDomain.some(i => typeof i === 'number')) {
scaler = colorScheme.createLinearScale();
} else {
scaler = colorScheme.createLinearScale(
Expand Down
5 changes: 2 additions & 3 deletions superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
import { isNumber } from 'lodash';
import { DataRecord, DTTM_ALIAS, ValueFormatter } from '@superset-ui/core';
import type { OptionName } from 'echarts/types/src/util/types';
import type { TooltipMarker } from 'echarts/types/src/util/format';
Expand Down Expand Up @@ -64,7 +63,7 @@ export const extractForecastValuesFromTooltipParams = (
const { marker, seriesId, value } = param;
const context = extractForecastSeriesContext(seriesId);
const numericValue = isHorizontal ? value[0] : value[1];
if (isNumber(numericValue)) {
if (typeof numericValue === 'number') {
if (!(context.name in values))
values[context.name] = {
marker: marker || '',
Expand Down Expand Up @@ -97,7 +96,7 @@ export const formatForecastTooltipSeries = ({
formatter: ValueFormatter;
}): string[] => {
const name = `${marker}${sanitizeHtml(seriesName)}`;
let value = isNumber(observation) ? formatter(observation) : '';
let value = typeof observation === 'number' ? formatter(observation) : '';
if (forecastTrend || forecastLower || forecastUpper) {
// forecast values take the form of "20, y = 30 (10, 40)"
// where the first part is the observation, the second part is the forecast trend
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* under the License.
*/
import { DataRecord, DataRecordValue } from '@superset-ui/core';
import { groupBy as _groupBy, isNumber, transform } from 'lodash';
import { groupBy as _groupBy, transform } from 'lodash';

export type TreeNode = {
name: DataRecordValue;
Expand All @@ -28,7 +28,7 @@ export type TreeNode = {
};

function getMetricValue(datum: DataRecord, metric: string) {
return isNumber(datum[metric]) ? (datum[metric] as number) : 0;
return typeof datum[metric] === 'number' ? (datum[metric] as number) : 0;
}

export function treeBuilder(
Expand Down
6 changes: 4 additions & 2 deletions superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ import {
PlusCircleOutlined,
TableOutlined,
} from '@ant-design/icons';
import { isEmpty, isNumber } from 'lodash';
import { isEmpty } from 'lodash';
import {
ColorSchemeEnum,
DataColumnMeta,
Expand Down Expand Up @@ -899,7 +899,9 @@ export default function TableChart<D extends DataRecord = DataRecord>(
/* The following classes are added to support custom CSS styling */
className={cx(
'cell-bar',
isNumber(value) && value < 0 ? 'negative' : 'positive',
typeof value === 'number' && value < 0
? 'negative'
: 'positive',
)}
css={cellBarStyles}
role="presentation"
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import { mountExploreUrl } from 'src/explore/exploreUtils';
import { postFormData } from 'src/explore/exploreUtils/formData';
import { URL_PARAMS } from 'src/constants';
import { SelectValue } from 'antd/lib/select';
import { isEmpty, isString } from 'lodash';
import { isEmpty } from 'lodash';

interface QueryDatabase {
id?: number;
Expand Down Expand Up @@ -280,7 +280,7 @@ export const SaveDatasetModal = ({
// Remove the special filters entry from the templateParams
// before saving the dataset.
let templateParams;
if (isString(datasource?.templateParams)) {
if (typeof datasource?.templateParams === 'string') {
const p = JSON.parse(datasource.templateParams);
Comment on lines +283 to 284
Copy link

@korbit-ai korbit-ai bot Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unhandled JSON Parse Error category Functionality

Tell me more
What is the issue?

The code doesn't handle JSON.parse errors which could occur if templateParams contains invalid JSON string.

Why this matters

If templateParams contains malformed JSON, the application will crash when trying to parse it, interrupting the save dataset operation.

Suggested change ∙ Feature Preview

Add try-catch block to handle potential JSON parsing errors:

let templateParams;
if (typeof datasource?.templateParams === 'string') {
  try {
    const p = JSON.parse(datasource.templateParams);
    /* eslint-disable-next-line no-underscore-dangle */
    if (p._filters) {
      /* eslint-disable-next-line no-underscore-dangle */
      delete p._filters;
      // eslint-disable-next-line no-param-reassign
      templateParams = JSON.stringify(p);
    }
  } catch (error) {
    console.error('Error parsing templateParams:', error);
    templateParams = datasource.templateParams;
  }
}
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.

Copy link
Contributor Author

@hainenber hainenber Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my change though

/* eslint-disable-next-line no-underscore-dangle */
if (p._filters) {
Expand Down
9 changes: 5 additions & 4 deletions superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import type {
CursorPosition,
} from 'src/SqlLab/types';
import type { DatabaseObject } from 'src/features/databases/types';
import { debounce, throttle, isBoolean, isEmpty } from 'lodash';
import { debounce, throttle, isEmpty } from 'lodash';
import Modal from 'src/components/Modal';
import Mousetrap from 'mousetrap';
import Button from 'src/components/Button';
Expand Down Expand Up @@ -281,9 +281,10 @@ const SqlEditor: FC<Props> = ({
if (unsavedQueryEditor?.id === queryEditor.id) {
dbId = unsavedQueryEditor.dbId || dbId;
latestQueryId = unsavedQueryEditor.latestQueryId || latestQueryId;
hideLeftBar = isBoolean(unsavedQueryEditor.hideLeftBar)
? unsavedQueryEditor.hideLeftBar
: hideLeftBar;
hideLeftBar =
typeof unsavedQueryEditor.hideLeftBar === 'boolean'
? unsavedQueryEditor.hideLeftBar
: hideLeftBar;
}
return {
hasSqlStatement: Boolean(queryEditor.sql?.trim().length > 0),
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import { useState } from 'react';
import fetchMock from 'fetch-mock';
import { omit, isUndefined, omitBy } from 'lodash';
import { omit, omitBy } from 'lodash';
import userEvent from '@testing-library/user-event';
import { waitFor, within } from '@testing-library/react';
import { render, screen } from 'spec/helpers/testing-library';
Expand Down Expand Up @@ -166,7 +166,7 @@ test('should generate Explore url', async () => {
form_data: {
...omitBy(
omit(formData, ['slice_id', 'slice_name', 'dashboards']),
isUndefined,
i => i === undefined,
),
groupby: ['name'],
adhoc_filters: [
Expand Down
3 changes: 1 addition & 2 deletions superset-frontend/src/explore/components/DatasourcePanel/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import { ControlConfig } from '@superset-ui/chart-controls';
import AutoSizer from 'react-virtualized-auto-sizer';
import { FixedSizeList as List } from 'react-window';

import { isArray } from 'lodash';
import { matchSorter, rankings } from 'match-sorter';
import Alert from 'src/components/Alert';
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
Expand Down Expand Up @@ -142,7 +141,7 @@ export default function DataSourcePanel({

const allowedColumns = useMemo(() => {
const validators = Object.values(dropzones);
if (!isArray(_columns)) return [];
if (!Array.isArray(_columns)) return [];
return _columns.filter(column =>
validators.some(validator =>
validator({
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/explore/components/controls/BoundsControl.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { useEffect, useRef, useState } from 'react';
import { InputNumber } from 'src/components/Input';
import { t, styled } from '@superset-ui/core';
import { debounce, parseInt } from 'lodash';
import { debounce } from 'lodash';
import ControlHeader from 'src/explore/components/ControlHeader';

type ValueType = (number | null)[];
Expand Down Expand Up @@ -47,7 +47,7 @@ const parseNumber = (value: undefined | number | string | null) => {
if (
value === null ||
value === undefined ||
(typeof value === 'string' && Number.isNaN(parseInt(value)))
(typeof value === 'string' && Number.isNaN(Number.parseInt(value, 10)))
) {
return null;
}
Expand Down
8 changes: 4 additions & 4 deletions superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
CategoricalColorNamespace,
} from '@superset-ui/core';
import AntdSelect from 'antd/lib/select';
import { isFunction, sortBy } from 'lodash';
import { sortBy } from 'lodash';
import ControlHeader from 'src/explore/components/ControlHeader';
import { Tooltip } from 'src/components/Tooltip';
import Icons from 'src/components/Icons';
Expand Down Expand Up @@ -163,7 +163,7 @@ const ColorSchemeControl = ({
}
let result = value || defaultScheme;
if (result === 'SUPERSET_DEFAULT') {
const schemesObject = isFunction(schemes) ? schemes() : schemes;
const schemesObject = typeof schemes === 'function' ? schemes() : schemes;
result = schemesObject?.SUPERSET_DEFAULT?.id;
}
return result;
Expand All @@ -179,8 +179,8 @@ const ColorSchemeControl = ({
</Option>,
];
}
const schemesObject = isFunction(schemes) ? schemes() : schemes;
const controlChoices = isFunction(choices) ? choices() : choices;
const schemesObject = typeof schemes === 'function' ? schemes() : schemes;
const controlChoices = typeof choices === 'function' ? choices() : choices;
const allColorOptions: string[] = [];
const filteredColorOptions = controlChoices.filter(o => {
const option = o[0];
Expand Down
3 changes: 1 addition & 2 deletions superset-frontend/src/explore/components/controls/DatasourceControl/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import ViewQueryModalFooter from 'src/explore/components/controls/ViewQueryModal
import ViewQuery from 'src/explore/components/controls/ViewQuery';
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
import { safeStringify } from 'src/utils/safeStringify';
import { isString } from 'lodash';
import { Link } from 'react-router-dom';

const propTypes = {
Expand Down Expand Up @@ -383,7 +382,7 @@ class DatasourceControl extends PureComponent {

let extra;
if (datasource?.extra) {
if (isString(datasource.extra)) {
if (typeof datasource.extra === 'string') {
try {
extra = JSON.parse(datasource.extra);
} catch {} // eslint-disable-line no-empty
Expand Down
3 changes: 1 addition & 2 deletions ...set-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
* specific language governing permissions and limitations
* under the License.
*/
import { isInteger } from 'lodash';
import { t, customTimeRangeDecode } from '@superset-ui/core';
import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
import { Col, Row } from 'src/components';
Expand Down Expand Up @@ -76,7 +75,7 @@ export function CustomFrame(props: FrameComponentProps) {
value: string | number,
) {
// only positive values in grainValue controls
if (isInteger(value) && value > 0) {
if (typeof value === 'number' && Number.isInteger(value) && value > 0) {
Copy link

@korbit-ai korbit-ai bot Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant Type Checking category Performance

Tell me more
What is the issue?

Multiple type checks are being performed unnecessarily in sequence, with Number.isInteger() already implying the value is a number.

Why this matters

The redundant type checking adds unnecessary computation overhead, especially in frequently executed code paths.

Suggested change ∙ Feature Preview

Simplify the condition to only use Number.isInteger() which inherently checks for number type:

if (Number.isInteger(value) && value > 0)
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.

Copy link
Contributor Author

@hainenber hainenber Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the typecheck in the beginning, there will be a type error

image

props.onChange(
customTimeRangeEncode({
...customRange,
Expand Down
13 changes: 6 additions & 7 deletions superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { render, screen } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { waitFor } from '@testing-library/react';
import { UploadFile } from 'antd/lib/upload/interface';
import { forEach } from 'lodash';

fetchMock.post('glob:*api/v1/database/1/csv_upload/', {});
fetchMock.post('glob:*api/v1/database/1/excel_upload/', {});
Expand Down Expand Up @@ -782,7 +781,7 @@ test('Columnar, form post', async () => {

test('CSV, validate file extension returns false', () => {
const invalidFileNames = ['out', 'out.exe', 'out.csv.exe', '.csv', 'out.xls'];
forEach(invalidFileNames, fileName => {
invalidFileNames.forEach(fileName => {
const file: UploadFile<any> = {
name: fileName,
uid: 'xp',
Expand All @@ -795,7 +794,7 @@ test('CSV, validate file extension returns false', () => {

test('Excel, validate file extension returns false', () => {
const invalidFileNames = ['out', 'out.exe', 'out.xls.exe', '.csv', 'out.csv'];
forEach(invalidFileNames, fileName => {
invalidFileNames.forEach(fileName => {
const file: UploadFile<any> = {
name: fileName,
uid: 'xp',
Expand All @@ -814,7 +813,7 @@ test('Columnar, validate file extension returns false', () => {
'.parquet',
'out.excel',
];
forEach(invalidFileNames, fileName => {
invalidFileNames.forEach(fileName => {
const file: UploadFile<any> = {
name: fileName,
uid: 'xp',
Expand All @@ -827,7 +826,7 @@ test('Columnar, validate file extension returns false', () => {

test('CSV, validate file extension returns true', () => {
const invalidFileNames = ['out.csv', 'out.tsv', 'out.exe.csv', 'out a.csv'];
forEach(invalidFileNames, fileName => {
invalidFileNames.forEach(fileName => {
const file: UploadFile<any> = {
name: fileName,
uid: 'xp',
Expand All @@ -840,7 +839,7 @@ test('CSV, validate file extension returns true', () => {

test('Excel, validate file extension returns true', () => {
const invalidFileNames = ['out.xls', 'out.xlsx', 'out.exe.xls', 'out a.xls'];
forEach(invalidFileNames, fileName => {
invalidFileNames.forEach(fileName => {
const file: UploadFile<any> = {
name: fileName,
uid: 'xp',
Expand All @@ -858,7 +857,7 @@ test('Columnar, validate file extension returns true', () => {
'out.exe.zip',
'out a.parquet',
];
forEach(invalidFileNames, fileName => {
invalidFileNames.forEach(fileName => {
const file: UploadFile<any> = {
name: fileName,
uid: 'xp',
Expand Down
17 changes: 5 additions & 12 deletions superset-frontend/src/reduxUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,7 @@
import { nanoid } from 'nanoid';
import { compose } from 'redux';
import persistState, { StorageAdapter } from 'redux-localstorage';
import {
isEqual,
omitBy,
omit,
isUndefined,
isNull,
isEqualWith,
} from 'lodash';
import { isEqual, omitBy, omit, isEqualWith } from 'lodash';
import { ensureIsArray } from '@superset-ui/core';

export function addToObject(
Expand Down Expand Up @@ -195,12 +188,12 @@ export function areObjectsEqual(
let comp1 = obj1;
let comp2 = obj2;
if (opts.ignoreUndefined) {
comp1 = omitBy(comp1, isUndefined);
comp2 = omitBy(comp2, isUndefined);
comp1 = omitBy(comp1, i => i === undefined);
comp2 = omitBy(comp2, i => i === undefined);
}
if (opts.ignoreNull) {
comp1 = omitBy(comp1, isNull);
comp2 = omitBy(comp2, isNull);
comp1 = omitBy(comp1, i => i === null);
comp2 = omitBy(comp2, i => i === null);
}
if (opts.ignoreFields?.length) {
const ignoreFields = ensureIsArray(opts.ignoreFields);
Expand Down
Loading