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

getEntityRecords() returns stale values when subsequent calls reduce the number of records #32594

Closed
johnbillion opened this issue Jun 10, 2021 · 3 comments
Labels
[Package] Core data /packages/core-data [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@johnbillion
Copy link
Member

Description

If you call select('core').getEntityRecords( .. ) with an argument which causes the number of results in the set to be reduced from its previous value, for example via the include parameter, unexpected stale values will be present at the end of the array.

Step-by-step reproduction instructions

See below for what seems to be the minimum code to reproduce the issue as a whole by implementing a filter on the Tags panel.

  1. Open the post editing screen and add a tag to the post
  2. This will trigger the withSelect() callback due to the change. ownProps.terms is an array containing the chosen tag ID. Observe the getEntityRecords() resolver fires off a REST API request to fetch the tag by using the include parameter on the endpoint.
  3. Add a second tag and observe the same, with both tag IDs in the include parameter.
  4. Remove one of the tags, thus causing the include parameter passed to getEntityRecords() to include one fewer elements that it did previously. Observe the resolver correctly queries for one term ID in the REST API request, but the subsequent return value of getEntityRecords() still contains two values.

What appears to be happening is the getEntityRecords() resolver iterates the values it receives from the REST API but somehow retains stale values for all subsequent elements in the array instead of discarding them.

Code snippet

This is a filter on the Tags panel which calls getEntityRecords() inside the withSelect() callback. You could manually call getEntityRecords() in your console but this demonstrates the problem within the context of withSelect().

const FilterTermSelector = () => {
  return (props) => {
    return (
      <>
        <TaxonomySelector {...props} />
        <CustomSelector {...props} />
      </>
    );
  };
};

wp.hooks.addFilter(
  'editor.PostTaxonomyType',
  'foo',
  FilterTermSelector,
);

class CustomSelector extends React.PureComponent {
  render() {
    return (
      <>
        <p>Terms: {JSON.stringify(this.props.terms)}</p>
        <p>Records: {JSON.stringify(this.props.records)}</p>
      </>
    );
  }
}

export default compose([
  withSelect((select, ownProps) => {
    const records = select('core').getEntityRecords('taxonomy', ownProps.taxonomy.slug, {
      include: ownProps.terms,
      _fields: 'id,name',
    });

    return {
      records,
    };
  }),
])(CustomSelector);

WordPress information

  • WordPress version: 5.7
  • Gutenberg version: N/A
@Mamaduka Mamaduka added the [Package] Core data /packages/core-data label Jun 11, 2021
@youknowriad youknowriad added [Type] Bug An existing feature does not function as intended Needs Testing Needs further testing to be confirmed. [Priority] High Used to indicate top priority items that need quick attention labels Jun 11, 2021
@Mamaduka
Copy link
Member

I think I was able to reproduce the issue with the code @johnbillion provided. However, it's a little inconsistent.

The displayed result happens in specific scenarios and not for all tag combinations.

I also came across this issue while working on #33459, but I couldn't reproduce it, so I ignored it.

Tags are generedet using WP CLI:

wp term generate post_tag --count=50

Screencast

CleanShot.2021-08-11.at.11.53.38.mp4

@Mamaduka Mamaduka removed the Needs Testing Needs further testing to be confirmed. label Aug 11, 2021
@Mamaduka
Copy link
Member

Mamaduka commented Sep 4, 2021

I was also able to reproduce this via the DevTools console. This method has the same results compared to UI reproduction steps.

Step by step

// Fetch first 10 items
wp.data.select('core').getEntityRecords('taxonomy', 'post_tag', { _fields: 'id,name' } );

// See the resolver fire request

// Fetch specific items by ID.
// The only requirement for `include` is that one of the items shouldn't be a part of the initial request.
wp.data.select('core').getEntityRecords('taxonomy', 'post_tag', { include: [ 5, 14 ], _fields: 'id,name' } );

// Wait for the resolver.

// Check the data. This will include item that was part of the initial response twice.
wp.data.select('core').getEntityRecords('taxonomy', 'post_tag', { include: [ 5, 14 ], _fields: 'id,name' } );

My guess

The source of the issue might be the getMergedItemIds. The following test will fail:

const original = deepFreeze( [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] );
const result = getMergedItemIds( original, [ 11, 4 ], 1, 10 );

expect( result ).toEqual( [ 11, 2, 3, 4, 5, 6, 7, 8, 9, 10 ] );

Is it expected for getMergedItemIds not to return unique IDs?

@Mamaduka
Copy link
Member

Mamaduka commented Sep 7, 2021

Fixed via #34583.

@Mamaduka Mamaduka closed this as completed Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Core data /packages/core-data [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants