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

Adds deprecation notice to useApiFetch hook #21723

Merged
merged 8 commits into from
Apr 21, 2020

Conversation

getdave
Copy link
Contributor

@getdave getdave commented Apr 20, 2020

This PR has been extracted from #21674.

As per [advice here from @aduth](See #21674 (comment)) this PR adds a deprecation notice to useApiFetch.

The work to remove this hook from usage within the Core Navigation Block is handled in a separate PR.

Description

Adds a deprecation warning to the useApiFetch hook within @wordpress/api-fetch.

Questions

How has this been tested?

  • Checkout this PR branch.
  • npm install && npm run dev
  • Create new Post.
  • Open browser devtools console drawer.
  • Insert the Core Navigation block.
  • See notice in console warning of usage of deprecated useApiFetch hook.

Please note: the work to remove this hook from usage within the Core Navigation Block is handled in a separate PR.

Screenshots

Screen Shot 2020-04-20 at 10 01 08

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions
Copy link

github-actions bot commented Apr 20, 2020

Size Change: -32 B (0%)

Total Size: 842 kB

Filename Size Change
build/annotations/index.js 3.62 kB +3 B (0%)
build/api-fetch/index.js 4.08 kB +72 B (1%)
build/block-library/index.js 112 kB +131 B (0%)
build/block-library/style-rtl.css 7.17 kB +5 B (0%)
build/block-library/style.css 7.18 kB +5 B (0%)
build/components/index.js 198 kB +7 B (0%)
build/edit-post/index.js 27.9 kB +1 B
build/edit-post/style-rtl.css 12.3 kB +54 B (0%)
build/edit-post/style.css 12.3 kB +55 B (0%)
build/edit-site/style-rtl.css 5.04 kB +5 B (0%)
build/edit-site/style.css 5.04 kB +6 B (0%)
build/edit-widgets/index.js 7.5 kB +5 B (0%)
build/edit-widgets/style-rtl.css 4.67 kB +5 B (0%)
build/edit-widgets/style.css 4.66 kB +5 B (0%)
build/editor/index.js 43.3 kB -1 B
build/editor/style-rtl.css 3.28 kB -198 B (6%)
build/editor/style.css 3.27 kB -198 B (6%)
build/element/index.js 4.65 kB +3 B (0%)
build/format-library/index.js 7.32 kB +2 B (0%)
build/nux/index.js 3.4 kB +1 B
build/primitives/index.js 1.49 kB +1 B
build/rich-text/index.js 14.8 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.24 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 761 B 0 B
build/block-editor/index.js 105 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.1 kB 0 B
build/block-library/editor.css 7.1 kB 0 B
build/block-library/theme-rtl.css 683 B 0 B
build/block-library/theme.css 685 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.7 kB 0 B
build/components/style-rtl.css 16.9 kB 0 B
build/components/style.css 16.9 kB 0 B
build/compose/index.js 6.66 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.25 kB 0 B
build/data/index.js 8.43 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.1 kB 0 B
build/edit-navigation/index.js 3.54 kB 0 B
build/edit-navigation/style-rtl.css 485 B 0 B
build/edit-navigation/style.css 485 B 0 B
build/edit-site/index.js 10.5 kB 0 B
build/editor/editor-styles-rtl.css 428 B 0 B
build/editor/editor-styles.css 431 B 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.67 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/server-side-render/index.js 2.67 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.02 kB 0 B
build/viewport/index.js 1.84 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

Copy link
Contributor

@marekhrabe marekhrabe left a comment

Choose a reason for hiding this comment

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

This works well for me and I'm able to see the deprecation notice in my console. It's incredibly long which might actually make it hard to observe but that has nothing to do with this usage.

Regarding the general direction to deprecate this feature, I'm deferring to the discussion in #21674 which looks like this approach is a go.

Screenshot 2020-04-20 at 12 33 02

@getdave getdave force-pushed the fix/deprecate-use-api-fetch-hook branch from 242e353 to c8d95dd Compare April 20, 2020 15:29
@getdave getdave closed this Apr 20, 2020
@getdave getdave reopened this Apr 20, 2020
@getdave
Copy link
Contributor Author

getdave commented Apr 20, 2020

@aduth if you find time to review this please note:

Backwards compatibility is supposed to be for x2 minor versions. Does that mean this hook is slated for removal in 7.11.0 or 8.1.0? I assume the former?

@aduth
Copy link
Member

aduth commented Apr 20, 2020

@aduth if you find time to review this please note:

Backwards compatibility is supposed to be for x2 minor versions. Does that mean this hook is slated for removal in 7.11.0 or 8.1.0? I assume the former?

Gutenberg follows WordPress's versioning conventions. There will be no 7.11.0. 8.0.0 comes after 7.9.0, so 8.1.0 would be the correct removal version.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

The build failure is likely because package-lock.json should be updated as a result of the change to the nested package.json (I believe it should be expected a one-line change, an addition of the package to requires section). Otherwise this looks good.

@getdave getdave self-assigned this Apr 21, 2020
@getdave getdave merged commit 7968fcb into master Apr 21, 2020
@getdave getdave deleted the fix/deprecate-use-api-fetch-hook branch April 21, 2020 13:34
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 21, 2020
@getdave
Copy link
Contributor Author

getdave commented Apr 22, 2020

@aduth Looks like I installed the dependency directly rather than via lerna. Didn't appreciate that workflow.

Trying to fix in #21780

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] API fetch /packages/api-fetch [Package] Deprecated packages/deprecated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants