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

[DataGrid] The api property passed on event publication / callbacks should always contain the apiRef.current, never apiRef #2189

Closed
1 task done
flaviendelangle opened this issue Jul 22, 2021 · 5 comments
Assignees
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module!
Milestone

Comments

@flaviendelangle
Copy link
Member

flaviendelangle commented Jul 22, 2021

Follow up #2187

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

The api property passed on event publication / callbacks should always contain the apiRef.current.

Examples 🌈

Currently, depending on which callback we are looking at, the api key can contain either the apiRef or the apiRef.current.

Sometimes, we give it apiRef

apiRef.current.publishEvent(GRID_COLUMN_VISIBILITY_CHANGE, {
  field,
  colDef: updatedCol,
  api: apiRef,
  isVisible,
});

Sometimes, we give it apiRef.current

const editRowParams: GridEditRowModelParams = {
  api: apiRef.current,
  model: apiRef.current.getState().editRows,
};
apiRef.current.publishEvent(GRID_ROW_EDIT_MODEL_CHANGE, editRowParams);

I'm in favor of doing it before beta because if we want to unify (which would be interesting I think), then it's a breaking change.

@flaviendelangle flaviendelangle added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 22, 2021
@flaviendelangle flaviendelangle self-assigned this Jul 22, 2021
@flaviendelangle flaviendelangle added breaking change component: data grid This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 22, 2021
@flaviendelangle flaviendelangle added this to the beta release milestone Jul 22, 2021
@m4theushw
Copy link
Member

The idea is to not pass the api in the params. It's reserved only to XGrid. In the features that are already using the useGridControlState hook it's not passed anymore.

@m4theushw
Copy link
Member

Related to #1821

@flaviendelangle
Copy link
Member Author

After migrating everything that we want to migrate to the useGridControlState hook, you think that their would be no callback with apiRef or apiRef.current ?

@dtassone
Copy link
Member

The idea is to not pass the api in the params. It's reserved only to XGrid. In the features that are already using the useGridControlState hook it's not passed anymore.

Todo this, you could check the license state refactored in #2176, and add a function in publishEvent that would filter it for you. I suggest finding a way to apply params.api = undefined transversally and not on the case by case basis. Otherwise, it won't scale and it would be much more prone to error

@flaviendelangle flaviendelangle changed the title [DataGrid] The api property passed on event publication / callbacks should always contain the apiRef.current. [DataGrid] The api property passed on event publication / callbacks should always contain the apiRef.current, never apiRef Jul 29, 2021
@DanailH
Copy link
Member

DanailH commented Aug 19, 2021

This is no longer relevant. ApiRef is removed from all callbacks but the one inside GridColDef. To use it one must access it through the details argument of the callback. The api is only available for DataGridPro

@DanailH DanailH closed this as completed Aug 19, 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!
Projects
None yet
Development

No branches or pull requests

4 participants