Skip to content

Conversation

@youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Aug 6, 2018

This PR removes the remaining usage of withAPIData from Gutenberg and officially deprecate its usage.

Testing instructions

  • Check that The lastest posts block is working properly
  • Check that you can set a parent page when creating pages.

closes #7397
closes #6277
closes #6379
closes #6537

@youknowriad youknowriad added the [Type] Code Quality Issues or PRs that relate to code quality label Aug 6, 2018
@youknowriad youknowriad self-assigned this Aug 6, 2018
@youknowriad youknowriad requested review from a team, aduth and gziolo August 6, 2018 09:19
order,
orderby: orderBy,
per_page: postsToShow,
_fields: [ 'date_gmt', 'link', 'title' ],
Copy link
Member

Choose a reason for hiding this comment

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

We are going to fetch more data, is that expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, with a central store like we do right now, we can't fetch a small part of objects, we fetch the whole objects or we don't. Because the redux state always expects the full object.

Copy link
Member

Choose a reason for hiding this comment

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

We could, but it is super complex :)

@gziolo
Copy link
Member

gziolo commented Aug 6, 2018

Eslint complains

/Users/gziolo/PhpstormProjects/gutenberg/core-blocks/latest-posts/edit.js
4:10 error 'get' is defined but never used no-unused-vars

Unit test fails:

● withAPIData() › should initialize fetchables on mount

expect(jest.fn()).not.toHaveWarned(expected)

Expected mock function not to be called but it was called with:
["wp.components.withAPIData is deprecated and will be removed from Gutenberg in 3.7. Please use the Core Data Module or wp.apiFetch directly instead."]

  26 |      afterEach( () => {
  27 |              if ( spy.assertionsNumber === 0 && spy.mock.calls.length > 0 ) {
> 28 |                      expect( console ).not[ matcherName ]();
     |                      ^
  29 |              }
  30 |      } );
  31 | };

  at Object.<anonymous> (packages/jest-console/src/index.js:28:4)

Otherwise everything works perfectly fine.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

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

Awesome 🎉

great one 💯

:shipit:

@youknowriad youknowriad merged commit 576872f into master Aug 6, 2018
@youknowriad youknowriad deleted the add/deprecate-with-api-data branch August 6, 2018 12:52
}

/**
* Returns the list of the taxonomies entities.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: The function returns a promise. The promise resolves to a list of the taxonomies entities.

Aside: This is probably one of the only things I dislike about async / await, that it becomes very non-obvious that it will always return a Promise (I think even if the function has no return statement? My uncertainty reenforces my point 😄 ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

4 participants