Fix update loops,

Fix linting Remove IdField property - DataGrid already has getRowId prop Minor fixes and tweaks
jamerst · Feb 13, 2022 · d99cf59 · d99cf59
1 parent 030be7e
commit d99cf59
Show file tree

Hide file tree

Showing 20 changed files with 165 additions and 151 deletions.
diff --git a/README.md b/README.md
@@ -74,14 +74,13 @@ _* = required property_
 | Name | Type | Default | Description |
 | ---- | ---- | ------- | ----------- |
 | `url*` | `string` | | URL of the OData endpoint |
-| `alwaysSelect` | `string[]` | | Array of entity properties to add to the `$select` clause of the query, even if a column doesn't exist for that property or the column is not visible. |
+| `alwaysSelect` | `string[]` | | Array of entity properties to add to the `$select` clause of the query, even if a column doesn't exist for that property or the column is not visible.<br/><br/>If you use the `getRowId` prop of the DataGrid, ensure that property is added here as well. |
 | `defaultPageSize` | `number` | `10` | The default page size to use. |
 | `defaultSortModel` | `GridSortModel` | | The default property/properties to sort by. |
 | `disableFilterBuilder` | `boolean` | | Disable the filter/query builder if set to `true` |
 | `disableHistory` | `boolean` | | Disable the browser history integration for sorting and pagination if set to `true`. <br/> **Note: this does not disable history integration for the filter builder.** |
 | `$filter` | `string` | | Static value to use for the `$filter` clause of the query.<br/><br/>**Note: this also has the effect of setting `disableFilterBuilder` to `true`**. |
 | `filterBuilderProps` | [`FilterBuilderProps`](#FilterBuilderProps) | | Props to be passed to the FilterBuilder. |
-| `idField` | `string` | | The DataGrid requires that each row in the grid has a property called `id`. If the unique ID field of your entity is not named `id`, provide the name of it here and the value of the property will be copied to `id` in the rows. |
 
 ### <a id="ODataGridColDef">ODataGridColDef</a>
 The column definition is again similar to the standard [GridColDef](https://mui.com/components/data-grid/columns/).
@@ -174,7 +173,7 @@ ODataGrid is developed using [pnpm](https://pnpm.io/). It will probably work fin
 To build and run the packages, you first need to install the development packages by running `pnpm i` in the `packages` directory. Once you have done that you can build or run the relevant package.
 
 ### Building
-Building is simple, just run `pnpm build` in the relevant directory under `packages`.
+Building is simple, just run `pnpm build` in `packages/o-data-grid` or `packages/o-data-grid-pro`.
 
 The build output is in the `build` directory.
 

diff --git a/packages/.eslintrc.json b/packages/.eslintrc.json
@@ -0,0 +1,32 @@
+{
+    "env": {
+        "browser": true,
+        "es2021": true
+    },
+    "extends": [
+        "eslint:recommended",
+        "plugin:react/recommended",
+        "plugin:@typescript-eslint/recommended"
+    ],
+    "parser": "@typescript-eslint/parser",
+    "parserOptions": {
+        "ecmaFeatures": {
+            "jsx": true
+        },
+        "ecmaVersion": 13,
+        "sourceType": "module"
+    },
+    "plugins": [
+        "react",
+        "react-hooks",
+        "@typescript-eslint"
+    ],
+    "rules": {
+        "react-hooks/rules-of-hooks": "error",
+        "react-hooks/exhaustive-deps": "warn",
+        "react/display-name": "off",
+        "react/prop-types": "off",
+        "@typescript-eslint/no-non-null-assertion": "off",
+        "@typescript-eslint/no-explicit-any": "off"
+    }
+}
diff --git a/packages/base/FilterBuilder/components/FilterCondition.tsx b/packages/base/FilterBuilder/components/FilterCondition.tsx
@@ -30,7 +30,7 @@ const FilterCondition = ({ clauseId, path }: FilterConditionProps) => {
     const newFieldDef = schema.find(c => c.field === newField);
 
     setClauses(old => old.update(clauseId, c => {
-      let condition = { ...c as ConditionClause };
+      const condition = { ...c as ConditionClause };
       condition.field = newField;
       condition.default = false;
 
@@ -76,7 +76,7 @@ const FilterCondition = ({ clauseId, path }: FilterConditionProps) => {
 
   const changeCollectionOp = useCallback((o: CollectionOperation) => {
     setClauses(old => old.update(clauseId, c => {
-      let condition = { ...c as ConditionClause, collectionOp: o, default: false };
+      const condition = { ...c as ConditionClause, collectionOp: o, default: false };
 
       // reset field operator if switching to count operator and current op is not valid
       if (o === "count" && !numericOperators.includes(condition.op)) {
@@ -91,7 +91,7 @@ const FilterCondition = ({ clauseId, path }: FilterConditionProps) => {
     const fieldDef = schema.find(c => c.field === field);
 
     setClauses(old => old.update(clauseId, c => {
-      let condition = { ...c as ConditionClause };
+      const condition = { ...c as ConditionClause };
       condition.collectionField = newColField;
       condition.default = false;
 
@@ -122,7 +122,7 @@ const FilterCondition = ({ clauseId, path }: FilterConditionProps) => {
         old.deleteIn([...path, clauseId]);
 
         // get path to parent node (i.e. remove "children" from end of path)
-        let parentPath = [...path];
+        const parentPath = [...path];
         parentPath.splice(-1, 1);
 
         do {

diff --git a/packages/base/FilterBuilder/components/FilterRoot.tsx b/packages/base/FilterBuilder/components/FilterRoot.tsx
@@ -9,7 +9,7 @@ import { clauseState, propsState, schemaState, treeState } from "../state"
 
 import { initialClauses, initialTree, rootConditionUuid, rootGroupUuid } from "../constants"
 import { FilterBuilderProps } from "./FilterBuilder";
-import { UseODataFilter } from "../hooks";
+import { UseODataFilter, UseODataFilterWithState } from "../hooks";
 import { useMountEffect } from "../../hooks";
 import { ConditionClause, SerialisedGroup, QueryStringCollection } from "../types";
 import { deserialise } from "../utils";
@@ -25,6 +25,7 @@ const FilterRoot = ({ props }: FilterRootProps) => {
   const setTree = useSetRecoilState(treeState);
 
   const odataFilter = UseODataFilter();
+  const odataFilterWithState = UseODataFilterWithState();
   const currentFilter = useRef("");
 
   const [anchor, setAnchor] = useState<null | HTMLElement>(null);
@@ -90,14 +91,9 @@ const FilterRoot = ({ props }: FilterRootProps) => {
   }, [props.schema, setClauses, setTree]);
 
   const restoreState = useCallback((state: any, isPopstate: boolean) => {
-    console.debug("restoreState");
-    if (!state) {
-      return;
-    }
-
     let filter = "", obj, queryString;
 
-    if (state.filterBuilder) {
+    if (state?.filterBuilder) {
       if (state.filterBuilder.reset === true && isPopstate === true) {
         restoreDefault();
       }
@@ -124,21 +120,19 @@ const FilterRoot = ({ props }: FilterRootProps) => {
   }, [onRestoreState, restoreDefault, setClauses, setTree]);
 
   const restoreFilter = useCallback((serialised: SerialisedGroup) => {
-    console.debug("restoreFilter");
     const [tree, clauses] = deserialise(serialised);
 
     setClauses(clauses);
     setTree(tree);
 
-    // if (onRestoreState) {
-    //   // const result = odataFilter();
+    if (onRestoreState) {
+      const result = odataFilterWithState(clauses, tree);
 
-    //   onRestoreState(result.filter ?? "", result.serialised, result.queryString);
-    // }
-  }, []);
+      onRestoreState(result.filter ?? "", result.serialised, result.queryString);
+    }
+  }, [setClauses, setTree, onRestoreState, odataFilterWithState]);
 
   useEffect(() => {
-    console.debug("popstate useEffect");
     if (disableHistory !== true) {
       const handlePopState = (e: PopStateEvent) => { restoreState(e.state, true); };
 
@@ -148,16 +142,14 @@ const FilterRoot = ({ props }: FilterRootProps) => {
   }, [disableHistory, restoreState]);
 
   useEffect(() => {
-    console.debug("propsFilter useEffect");
     if (propsFilter) {
       restoreFilter(propsFilter);
     } else {
       restoreDefault();
     }
-  }, [propsFilter, restoreFilter]);
+  }, [propsFilter, restoreFilter, restoreDefault]);
 
   useMountEffect(() => {
-    console.debug("useMountEffect");
     setProps(props);
 
     // restore query from history state if enabled

diff --git a/packages/base/FilterBuilder/hooks.ts b/packages/base/FilterBuilder/hooks.ts
@@ -13,6 +13,14 @@ export const UseODataFilter = () => {
   }, [schema, clauses, tree]);
 }
 
+export const UseODataFilterWithState = () => {
+  const schema = useRecoilValue(schemaState);
+
+  return useCallback((clauses: StateClause, tree: StateTree) => {
+    return buildGroup(schema, clauses, tree, rootGroupUuid, []) as BuiltQuery<SerialisedGroup>;
+  }, [schema])
+}
+
 type BuiltInnerQuery = {
   filter?: string,
   queryString?: QueryStringCollection

diff --git a/packages/base/components/ODataGridBase.tsx b/packages/base/components/ODataGridBase.tsx
@@ -6,7 +6,7 @@ import { ResponsiveValues, useResponsive } from "../hooks";
 
 import FilterBuilder from "../FilterBuilder/components/FilterBuilder";
 
-import { Expand, ODataGridBaseColDef, ODataResponse, ODataGridBaseProps, IGridSortModel, IGridProps, IGridRowModel, ColumnVisibilityModel } from "../types";
+import { Expand, ODataResponse, ODataGridBaseProps, IGridSortModel, IGridProps, IGridRowModel, ColumnVisibilityModel } from "../types";
 
 import { ExpandToQuery, Flatten, GroupArrayBy, GetPageNumber, GetPageSizeOrDefault } from "../utils";
 
@@ -28,12 +28,10 @@ const ODataGridBase = <ComponentProps extends IGridProps,
   const [queryString, setQueryString] = useState<QueryStringCollection | undefined>();
 
   const [visibleColumns, setVisibleColumns] = useState<string[]>(props.columns
-    .filter(c => (props.columnVisibilityModel && props.columnVisibilityModel[c.field] !== true) || c.hide !== true)
+    .filter(c => (props.columnVisibilityModel && props.columnVisibilityModel[c.field] !== false) || c.hide !== true)
     .map(c => c.field)
   );
-  const [columnVisibility, setColumnVisibility] = useState<ColumnVisibilityModel>({});
   const [columnVisibilityOverride, setColumnVisibilityOverride] = useState<ColumnVisibilityModel>({});
-  const [columnHideOverrides, setColumnHideOverrides] = useState<{ [key: string]: boolean }>({});
 
   const firstLoad = useRef<boolean>(true);
   const fetchCount = useRef<boolean>(true);
@@ -64,11 +62,6 @@ const ODataGridBase = <ComponentProps extends IGridProps,
         .map(c => c.select ?? c.field)
     );
 
-    // add id field if specified
-    if (props.idField) {
-      fields.add(props.idField);
-    }
-
     if (props.alwaysSelect) {
       props.alwaysSelect.forEach((c) => fields.add(c));
     }
@@ -98,7 +91,7 @@ const ODataGridBase = <ComponentProps extends IGridProps,
     const $top = pageSize;
     const $skip = pageNumber * pageSize;
 
-    let query: OdataQuery = {
+    const query: OdataQuery = {
       $select,
       $expand,
       $top,
@@ -127,17 +120,11 @@ const ODataGridBase = <ComponentProps extends IGridProps,
     const response = rawResponse as Response;
 
     if (response?.ok ?? false) {
-      let data = await response.json() as ODataResponse;
+      const data = await response.json() as ODataResponse;
 
       // flatten object so that the DataGrid can access all the properties
       // i.e. { Person: { name: "John" } } becomes { "Person/name": "John" }
-      let rows = data.value.map(v => Flatten(v, "/"));
-
-      // extract id if data does not contain the "id" field already
-      // DataGrid requires each row to have a unique "id" property
-      if (props.idField) {
-        rows = rows.map(r => { return { ...r, id: r[props.idField!] } });
-      }
+      const rows = data.value.map(v => Flatten(v, "/"));
 
       if (data["@odata.count"]) {
         setRowCount(data["@odata.count"]);
@@ -160,7 +147,6 @@ const ODataGridBase = <ComponentProps extends IGridProps,
       filter,
       queryString,
       props.url,
-      props.idField,
       props.alwaysSelect,
       props.columns,
       props.$filter,
@@ -193,7 +179,7 @@ const ODataGridBase = <ComponentProps extends IGridProps,
     }
 
     if (props.disableHistory !== true) {
-      if (state.oDataGrid?.sortModel) {
+      if (state?.oDataGrid?.sortModel) {
         setSortModel(state.oDataGrid.sortModel as SortModel);
       } else {
         setSortModel(props.defaultSortModel);
@@ -224,33 +210,6 @@ const ODataGridBase = <ComponentProps extends IGridProps,
     }
   }, [onColumnVisibilityChange]);
 
-  useEffect(() => {
-    let visibility: ColumnVisibilityModel = {};
-    if (props.columnVisibilityModel) {
-      for (const field in props.columnVisibilityModel) {
-        if (typeof props.columnVisibilityModel[field] === "boolean") {
-          visibility[field] = props.columnVisibilityModel[field] as boolean;
-        } else {
-          visibility[field] = r(props.columnVisibilityModel[field] as ResponsiveValues<boolean>) as boolean;
-        }
-      }
-    } else {
-      props.columns.filter(c => c.filterOnly !== true).forEach(c => {
-        if (typeof c.hide === "boolean") {
-          visibility[c.field] = !(c.hide as boolean);
-        } else if (c.hide) {
-          visibility[c.field] = !r(c.hide as ResponsiveValues<boolean>);
-        }
-      })
-    }
-
-    props.columns.filter(c => c.filterOnly === true).forEach(c => {
-      visibility[c.field] = false;
-    })
-
-    setColumnVisibility(visibility);
-  }, [r, props.columnVisibilityModel, props.columns]);
-
   const handleSortModelChange = useCallback((model: SortModel, details) => {
     if (onSortModelChange) {
       onSortModelChange(model, details);
@@ -346,7 +305,7 @@ const ODataGridBase = <ComponentProps extends IGridProps,
       if (props.disableHistory !== true && props.disableFilterBuilder === true) {
         // only restore sort model from history if history is enabled and FilterBuilder is disabled
         // if FilterBuilder is enabled sort model restoration is handled in handleBuilderRestore
-        if (e.state.oDataGrid?.sortModel) {
+        if (e.state?.oDataGrid?.sortModel) {
           setSortModel(e.state.oDataGrid.sortModel as SortModel);
         } else {
           setSortModel(props.defaultSortModel);
@@ -366,11 +325,39 @@ const ODataGridBase = <ComponentProps extends IGridProps,
     setPageSize(size);
   }, []);
 
-  // combine columnVisibility and columnVisibilityOverride
-  // columnVisibilityOverride will take priority if the same field is in both
+
   const visibility = useMemo(
-    () => ({ ...columnVisibility, ...columnVisibilityOverride }),
-    [columnVisibility, columnVisibilityOverride]
+    () => {
+      const v: ColumnVisibilityModel = {};
+      if (props.columnVisibilityModel) {
+        for (const field in props.columnVisibilityModel) {
+          if (field in columnVisibilityOverride) {
+            v[field] = columnVisibilityOverride[field];
+          } else if (typeof props.columnVisibilityModel[field] === "boolean") {
+            v[field] = props.columnVisibilityModel[field] as boolean;
+          } else {
+            v[field] = r(props.columnVisibilityModel[field] as ResponsiveValues<boolean>) as boolean;
+          }
+        }
+      } else {
+        props.columns.filter(c => c.filterOnly !== true).forEach(c => {
+          if (c.field in columnVisibilityOverride) {
+            v[c.field] = columnVisibilityOverride[c.field];
+          } else if (typeof c.hide === "boolean") {
+            v[c.field] = !(c.hide as boolean);
+          } else if (c.hide) {
+            v[c.field] = !r(c.hide as ResponsiveValues<boolean>);
+          }
+        })
+      }
+
+      props.columns.filter(c => c.filterOnly === true).forEach(c => {
+        v[c.field] = false;
+      })
+
+      return v;
+    },
+    [props.columnVisibilityModel, r, props.columns, columnVisibilityOverride]
   );
 
   const GridComponent = props.component;

diff --git a/packages/base/hooks.ts b/packages/base/hooks.ts
@@ -29,7 +29,7 @@ export const useBreakpoints = ():Partial<Record<Breakpoint, boolean>> => {
 
   useEffect(() => {
     const queries = getQueries(theme.breakpoints.keys, theme);
-    let listeners: Partial<Record<Breakpoint, () => void>> = {};
+    const listeners: Partial<Record<Breakpoint, () => void>> = {};
 
     const updateMatch = (b: Breakpoint) => {
       setMatches((oldMatches) => ({...oldMatches, [b]: queries[b]?.matches ?? false }));

diff --git a/packages/base/types.ts b/packages/base/types.ts
@@ -21,8 +21,7 @@ export type ODataGridBaseProps<
     disableFilterBuilder?: boolean,
     disableHistory?: boolean,
     $filter?: string,
-    filterBuilderProps?: ExternalBuilderProps,
-    idField?: string,
+    filterBuilderProps?: ExternalBuilderProps
   };
 
 // remove properties which should not be used - these are handled internally or overridden