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

API: Replace all withAPIData usage and deprecate the HoC #8584

Merged
merged 3 commits into from
Aug 6, 2018

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 gziolo, aduth and a team August 6, 2018 09:19
const latestPostsQuery = pickBy( {
categories,
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
@@ -40,6 +41,22 @@ async function loadPostTypeEntities() {
} );
}

/**
* 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
3 participants