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

Fix List view error after delete when using a field with no record test #5900

Merged
merged 7 commits into from
Feb 12, 2021

Conversation

fzaninotto
Copy link
Member

@fzaninotto fzaninotto commented Feb 11, 2021

This fixes a nasty bug that occurs when:

  • A list contains a Field that doesn't check if the record is defined before rendering it (e.g. FooField = ({ record }) => <>{record.foo}</> instead of FooField = ({ record }) => <>{record ? record.foo : null}</>
  • The user deletes a record from the list using bulk actions

image

In addition, this bug caused an additional List rerender, so this should also speed up lists a bit.

Note to self: never use both a dataProvider hook and a useSelector in the same function. Because dataProvider hooks rely on useEffect, they will return one tick later than useSelector, resulting in data inconsistencies (in this case: a list of ids containing the id of a record already removed from the data key).

@fzaninotto fzaninotto added the RFR Ready For Review label Feb 11, 2021
@fzaninotto fzaninotto added this to the 3.12.2 milestone Feb 11, 2021
@@ -41,6 +42,73 @@ describe('useListController', () => {
debounce: 200,
};

describe('data', () => {
it('should be synchronized with ids after delete', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

this test fails on master

@@ -181,41 +178,12 @@ const useListController = <RecordType extends Record = Record>(
}
);

const data = useSelector(
Copy link
Member Author

Choose a reason for hiding this comment

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

Logic moved to a special hook, useGetMainList

@djhi djhi merged commit eb63c7a into master Feb 12, 2021
@djhi djhi deleted the fix-useListController-data-ids branch February 12, 2021 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants