Skip to content
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.

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

Codify lodash usage in eslint #1270

Merged
merged 13 commits into from
Jul 13, 2023
15 changes: 15 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const baseline = require('@mui/monorepo/.eslintrc');
const lodash = require('lodash');

const ALLOWED_LODASH_METHODS = new Set(['throttle', 'debounce', 'set']);

module.exports = {
...baseline,
Expand All @@ -25,6 +28,18 @@ module.exports = {
name: '@mui/icons-material',
message: 'Use @mui/icons-material/<Icon> instead.',
},
{
name: 'lodash-es',
importNames: Object.keys(lodash).filter((key) => !ALLOWED_LODASH_METHODS.has(key)),
message:
'Avoid kitchensink libraries like lodash-es. We prefer a slightly more verbose, but more universally understood javascript style',
},
],
patterns: [
{
group: ['lodash-es/*'],
message: 'Use `import { debounce } from "lodash-es";` instead.',
},
],
},
],
Expand Down
9 changes: 0 additions & 9 deletions packages/toolpad-app/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,6 @@ export const DOCUMENTATION_INSTALLATION_URL =
export const ROADMAP_URL = 'https://github.com/orgs/mui/projects/9';
export const SCHEDULE_DEMO_URL = 'https://calendly.com/prakhar-mui/toolpad';

export const PRODUCTION_DATASOURCES = new Set([
'rest',
'function',
'googleSheets',
'postgres',
'mysql',
'local',
]);

export const TOOLPAD_BRIDGE_GLOBAL = '__TOOLPAD_BRIDGE__';

export const NON_BINDABLE_CONTROL_TYPES = ['GridColumns'];
Expand Down
21 changes: 10 additions & 11 deletions packages/toolpad-app/src/runtime/ToolpadApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import {
JsExpressionAttrValue,
} from '@mui/toolpad-core';
import { createProvidedContext, useAssertedContext } from '@mui/toolpad-utils/react';
import { mapProperties, mapValues } from '@mui/toolpad-utils/collections';
import { set as setObjectPath } from 'lodash-es';
Copy link
Member

Choose a reason for hiding this comment

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

If we plan to eventually completely get rid of lodash we could maybe create utility methods for these operations, which for now would still use lodash but then we would know where to replace it.
Just a possibility, maybe it helps to have the lodash imports less spread out.

Copy link
Member Author

@Janpot Janpot Jul 13, 2023

Choose a reason for hiding this comment

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

To be honest, I'm not sure we should even have utility functions for this. ideally we improve the algorithm altogether to be not so lodashy. setObjectPath also makes us write type-unsafe code, I don't want to enable that sort of code in our codebase

Copy link
Member

Choose a reason for hiding this comment

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

Got it, yeah for this specific type of operation maybe just normal Javascript would be possible and be more type-safe.

import { QueryClient, QueryClientProvider, useMutation } from '@tanstack/react-query';
import {
BrowserRouter,
Expand All @@ -52,12 +54,10 @@ import {
NodeRuntimeWrapper,
ResetNodeErrorsKeyProvider,
} from '@mui/toolpad-core/runtime';
import * as _ from 'lodash-es';
import ErrorIcon from '@mui/icons-material/Error';
import { getBrowserRuntime } from '@mui/toolpad-core/jsBrowserRuntime';
import * as builtIns from '@mui/toolpad-components';
import { errorFrom } from '@mui/toolpad-utils/errors';
import { mapProperties, mapValues } from '@mui/toolpad-utils/collections';
import useBoolean from '@mui/toolpad-utils/hooks/useBoolean';
import usePageTitle from '@mui/toolpad-utils/hooks/usePageTitle';
import invariant from 'invariant';
Expand All @@ -79,7 +79,7 @@ import evalJsBindings, {
} from './evalJsBindings';
import { HTML_ID_EDITOR_OVERLAY, NON_BINDABLE_CONTROL_TYPES } from './constants';
import { layoutBoxArgTypes } from './toolpadComponents/layoutBox';
import { execDataSourceQuery, useDataQuery, UseDataQueryConfig, UseFetch } from './useDataQuery';
import { execDataSourceQuery, useDataQuery, UseFetch } from './useDataQuery';
import { NavigateToPage } from './CanvasHooksContext';
import AppNavigation from './AppNavigation';
import PreviewHeader from './PreviewHeader';
Expand Down Expand Up @@ -134,11 +134,6 @@ const INITIAL_FETCH: UseFetch = {
rows: [],
};

const USE_DATA_QUERY_CONFIG_KEYS: readonly (keyof UseDataQueryConfig)[] = [
'enabled',
'refetchInterval',
];

function usePageNavigator(): NavigateToPage {
const navigate = useNavigate();

Expand Down Expand Up @@ -354,7 +349,7 @@ function resolveBindables(
return { loading: true };
}

_.set(result, `${resultKey}${path}`, resolvedBinding?.value);
setObjectPath(result, `${resultKey}${path}`, resolvedBinding?.value);
}

return { value: result[resultKey] || {} };
Expand Down Expand Up @@ -428,6 +423,10 @@ function getScopeElements(
return result;
}

function getQueryConfigBindings({ enabled, refetchInterval }: appDom.QueryNode['attributes']) {
return { enabled, refetchInterval };
}

function parseBindings(
dom: appDom.AppDom,
rootNode: appDom.ElementNode | appDom.PageNode | appDom.ElementNode[],
Expand Down Expand Up @@ -562,7 +561,7 @@ function parseBindings(
});
}

const configBindings = _.pick(elm.attributes, USE_DATA_QUERY_CONFIG_KEYS);
const configBindings = getQueryConfigBindings(elm.attributes);
const nestedBindablePaths = flattenNestedBindables(configBindings);

for (const [nestedPath, paramValue] of nestedBindablePaths) {
Expand Down Expand Up @@ -1226,7 +1225,7 @@ function QueryNode({ page, node }: QueryNodeProps) {
Object.fromEntries(node.params ?? []),
);

const configBindings = _.pick(node.attributes, USE_DATA_QUERY_CONFIG_KEYS);
const configBindings = getQueryConfigBindings(node.attributes);
const options = resolveBindables(bindings, `${node.id}.config`, configBindings);

const inputError = params.error || options.error;
Expand Down
4 changes: 2 additions & 2 deletions packages/toolpad-app/src/runtime/evalJsBindings.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BindingEvaluationResult, JsRuntime } from '@mui/toolpad-core';
import { set } from 'lodash-es';
import { set as setObjectPath } from 'lodash-es';
import { mapValues } from '@mui/toolpad-utils/collections';
import { updatePath } from '../utils/immutability';

Expand Down Expand Up @@ -110,7 +110,7 @@ export function buildGlobalScope(
for (const binding of Object.values(bindings)) {
if (binding.scopePath) {
const value = binding.result?.value;
set(globalScope, binding.scopePath, value);
setObjectPath(globalScope, binding.scopePath, value);
}
}
return globalScope;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Stack, styled, Typography, Divider } from '@mui/material';
import * as React from 'react';
import * as _ from 'lodash-es';
// TODO: Remove lodash-es import here
// eslint-disable-next-line no-restricted-imports
import { groupBy } from 'lodash-es';
import {
ArgTypeDefinition,
ArgTypeDefinitions,
Expand Down Expand Up @@ -98,7 +100,7 @@ function ComponentPropsEditor<P extends object>({
);
}, [bindings, node.id]);

const argTypesByCategory = _.groupBy(
const argTypesByCategory: Record<string, ExactEntriesOf<ArgTypeDefinitions<P>>> = groupBy(
Object.entries(componentConfig.argTypes || {}) as ExactEntriesOf<ArgTypeDefinitions<P>>,
([, propTypeDef]) => propTypeDef?.category || 'properties',
);
Expand Down
1 change: 1 addition & 0 deletions packages/toolpad-app/src/toolpad/AppState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { NodeId } from '@mui/toolpad-core';
import { createProvidedContext } from '@mui/toolpad-utils/react';
import invariant from 'invariant';
import { debounce, DebouncedFunc } from 'lodash-es';

import { useLocation } from 'react-router-dom';
import { mapValues } from '@mui/toolpad-utils/collections';
import * as appDom from '../appDom';
Expand Down
8 changes: 2 additions & 6 deletions packages/toolpad-app/src/toolpadDataSources/client.tsx
Original file line number Diff line number Diff line change
@@ -1,22 +1,18 @@
import * as _ from 'lodash-es';
import postgres from './postgres/client';
import mysql from './mysql/client';
import rest from './rest/client';
import { ClientDataSource } from '../types';
import googleSheets from './googleSheets/client';
import local from './local/client';
import { PRODUCTION_DATASOURCES } from '../constants';

type ClientDataSources = { [key: string]: ClientDataSource<any, any> | undefined };

export const allClientDataSources: ClientDataSources = {
const dataSources: ClientDataSources = {
rest,
postgres,
googleSheets,
mysql,
local,
};

const clientDataSources = _.pick(allClientDataSources, [...PRODUCTION_DATASOURCES]);

export default clientDataSources;
export default dataSources;
22 changes: 8 additions & 14 deletions packages/toolpad-app/src/toolpadDataSources/server.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,18 @@
import * as _ from 'lodash-es';
import { ServerDataSource } from '../types';
import postgres from './postgres/server';
import mysql from './mysql/server';
import rest from './rest/server';
import googleSheets from './googleSheets/server';
import local from './local/server';

import { PRODUCTION_DATASOURCES } from '../constants';

type ServerDataSources = { [key: string]: ServerDataSource<any, any, any> | undefined };

const serverDataSources: ServerDataSources = _.pick(
{
rest,
postgres,
googleSheets,
mysql,
local,
},
[...PRODUCTION_DATASOURCES],
);
const dataSources: ServerDataSources = {
rest,
postgres,
googleSheets,
mysql,
local,
};

export default serverDataSources;
export default dataSources;
2 changes: 2 additions & 0 deletions packages/toolpad-app/src/utils/immutability.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// TODO: remove these lodash-es imports
// eslint-disable-next-line no-restricted-imports
import { setWith, clone } from 'lodash-es';

/**
Expand Down
6 changes: 4 additions & 2 deletions packages/toolpad-components/src/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { Container, ContainerProps, Box, Stack, BoxProps } from '@mui/material';
import { LoadingButton } from '@mui/lab';
import { ArgTypeDefinitions, createComponent, useNode } from '@mui/toolpad-core';
import { useForm, FieldValues, ValidationMode, FieldError, Controller } from 'react-hook-form';
import * as _ from 'lodash-es';
// TODO: Remove lodash-es
// eslint-disable-next-line no-restricted-imports
import { isEqual } from 'lodash-es';
import { SX_PROP_HELPER_TEXT } from './constants';

export const FormContext = React.createContext<{
Expand Down Expand Up @@ -257,7 +259,7 @@ export function useFormInput<V>({
React.useEffect(() => {
if (
form &&
!_.isEqual(validationProps, previousManualValidationPropsRef.current) &&
!isEqual(validationProps, previousManualValidationPropsRef.current) &&
form.formState.dirtyFields[formInputName]
) {
form.trigger(formInputName);
Expand Down