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

[core] Each hook should initialize its state synchronously #2782

Merged
merged 94 commits into from
Oct 11, 2021

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Oct 5, 2021

Fixes #2293
Part of #924

Overview

  • Each hook is now responsible for its state initialization
  • The components should never import selectors from optional hook like useGridColumnReorder, useGridColumnResize and useGridInfiniteLoader. Instead, the feature hook should inject the props in props.componentsProps?.XXX
  • The state initialization should be synchronous except for DOM-related information
  • Unify model update behaviors : always check if the new model equals the old one before storing it in the state

I plan to rework the folder structure in a future PR to put back the pro-only features inside the data-grid-pro folder to be able to ban the imports from data-grid to data-grid-pro with ESLint.

Breaking changes

Merge state.visibleRows and state.filter into state.filter

- state.filter
- gridFilterStateSelector(state)
+ state.filter.filterModel
+ gridFilterModelSelector(state) // preferred method

- state.visibleRows.visibleRowsLookup
- visibleGridRowsStateSelector(state).visibleRowsLookup
+ state.filter.visibleRowsLookup
+ gridVisibleRowsLookupSelector(state).visibleRowsLookup // preferred method

- state.visibleRows.visibleRows
+ state.filter.visibleRows
+ gridVisibleRowsLookupSelector(state).visibleRows // preferred method

Rename getInitialGridFilterState into getDefaultGridFilterModel

-const [filterModel, setFilterModel] = React.useState(getInitialGridFilterState);
+const [filterModel, setFilterModel] = React.useState(getDefaultGridFilterModel);

Remove getInitialGridSortingState

-const [sortModel, setSortModel] = React.useState(() => getInitialGridSortingState().sortModel);
+const [sortModel, setSortModel] React.useState([]);

Remove other initialize method

-getInitialGridColumnReorderState
-getInitialGridColumnResizeState
-getInitialGridColumnsState
-getInitialGridRenderingState
-getInitialGridRowState
-getInitialGridState
-getInitialVisibleGridRowsState

Detailed structure of a feature hook

I did not reorder the elements of every feature hooks because the order does not always cause bugs.
For instance, if you instantiate the api methods after the events listeners, it only cause problems if one of the event listeners is an api method (ex: useGridApiEventHandler(apiRef, GridEvents.rowsSet, apiRef.current.applyFilters);).

But we should use the following order as much as possible to avoid any issue.

Concerning the hook order, the following hook should be after useGridSomeOtherFeature as it uses its state in the render phase.
If possible it should be after useGridRows as it depends on it, but if for some reason you have to put it after, it would work.
The dependance is asynchronous (the GridEvents.rowsSet is published through a useEffect) so useGridExample would already be initialized for the 1st event publication even if its after in the feature list).

/**
 * @requires useGridSomeOtherFeature (state)
 * @requires useGridRows (event)
 */
export const useGridExample = (apiRef: GridApiRef, props: GridComponentProps) => {
  // 1. LOGGER
  const logger = useGridLogger(apiRef, 'useGridExample');

  // 2. STATE INITIALISATION
  useGridStateInit(apiRef, (state) => ({
    ...state,
    example: {
      exampleModel: props.exampleModel ?? getDefaultExampleModel(),
    },
  }));

  // 3. STATE SELECTORS AND UPDATER
  const [, setGridState, forceUpdate] = useGridState(apiRef);
  const someOtherFeatureSubState = useGridSelector(apiRef, gridSomeOtherFeatureSubSelector);


  // 4. SYNCHRONOUS CONTROL STATE REGISTRATION
  useGridRegisterControlState(apiRef, {
    stateId: 'example',
    propModel: props.exampleModel,
    propOnChange: props.onExampleModelChange,
    stateSelector: gridExampleModelSelector,
    changeEvent: GridEvents.exampleChange,
  });

  // 5. API METHODS
  const setExampleModel = React.useCallback<GridExampleApi['setExampleModel']>(
    (model) => {
      const currentModel = gridExampleModelSelector(apiRef.current.state);

      if (currentModel !== model) {
        logger.debug('Setting example model');
  
        setGridState((state) => ({
          ...state,
          example: {
            ...state.example,
            exampleModel: model,
          },
        }));

        // If we have complex post update logic to run after model update, we must call it here
        // If not, remember to call `forceUpdate` here
        apiRef.current.applyExample();
      }
    },
    [apiRef, setGridState, logger],
  );

  const applyExample = React.useCallback<GridExampleApi['applyExample']>(
    () => { /* Some code to run whenever the model or the rows are updated */ },
    []
  )

  const exampleApi: GridExampleApi = {
    setExampleModel,
    applyExample,
  };

  useGridApiMethod(apiRef, exampleApi, 'GridExampleApi');

  // 6. UPDATE EFFECTS
  React.useEffect(() => {
    if (props.exampleModel != null) {
      apiRef.current.setExampleModel(props.exampleModel);
    }
  }, [apiRef, props.exampleModel]);

  // 7. FIRST RUN COMPLEX STATE INITIALIZATION
  // Some logic for the state initialization requires to much of the hook codebase to be placed inside `useGridStateInit`.
  //  We add a callback here to run it synchronously during the 1st render, but after the initialization of the api methods.
  useFirstRender(() => apiRef.current.applyExample());

  // 8. EVENT CALLBACKS
  const handleColumnHeaderClick = React.useCallback(
    () => { /* Some code to run when the user clicks on a column header */ },
    [],
  );

  useGridApiEventHandler(apiRef, GridEvents.rowsSet, this.apiRef.applyExample);
  useGridApiEventHandler(apiRef, GridEvents.columnHeaderClick, handleColumnHeaderClick);
};

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 5, 2021
@github-actions
Copy link

github-actions bot commented Oct 5, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 5, 2021
@flaviendelangle flaviendelangle marked this pull request as ready for review October 5, 2021 15:00
@github-actions
Copy link

github-actions bot commented Oct 5, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 5, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 6, 2021
@flaviendelangle flaviendelangle self-assigned this Oct 6, 2021
@flaviendelangle flaviendelangle added breaking change component: data grid This is the name of the generic UI component, not the React module! core Infrastructure work going on behind the scenes labels Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[core] Each feature hook should initialize synchronously its state
1 participant