Skip to content

/ gutenberg

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

Jump to bottom

Adds deprecation notice to useApiFetch hook #21723

Merged
merged 8 commits into from Apr 21, 2020
Merged

Adds deprecation notice to useApiFetch hook #21723

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.
@getdave getdave requested review from mmtr and nerrad as code owners Apr 20, 2020
This was referenced Apr 20, 2020
Closed
Merged
@github-actions
Copy link

@github-actions github-actions bot commented Apr 20, 2020 •
edited

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

@marekhrabe
marekhrabe approved these changes Apr 20, 2020
View changes
Copy link
Contributor

@marekhrabe marekhrabe left a comment

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 requested review from ajitbohra, ntwb and talldan as code owners Apr 20, 2020
getdave added 4 commits Apr 20, 2020
@getdave
Adds deprecation notice to useApiFetch hook
fce9f96 
See #21674 (comment)
@getdave
Simplify deprecation message
8878ccb
@getdave
Fix e2e tests by expecting deprecation notice
c15b1d8
@getdave
Fix useApiFetch warning in related tests
c8d95dd
@getdave getdave force-pushed the fix/deprecate-use-api-fetch-hook branch from 242e353 to c8d95dd Apr 20, 2020
@getdave getdave closed this Apr 20, 2020
@getdave getdave reopened this Apr 20, 2020
@getdave
Copy link
Contributor Author

@getdave 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 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.

@aduth
aduth approved these changes Apr 20, 2020
View changes
Copy link
Member

@aduth aduth left a comment

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 added 2 commits Apr 21, 2020
@getdave
Update lockfile
cbcadbe
@getdave
Update warning to use correct deprecation version
77b8ccd
@getdave getdave self-assigned this Apr 21, 2020
getdave added 2 commits Apr 21, 2020
@getdave
Update version ref in tests
5a2b7e6
@getdave
Fix remaining versions refs in tests
fdff95c
@getdave getdave merged commit 7968fcb into master Apr 21, 2020
3 checks passed
3 checks passed
@github-actions
build
Details
@github-actions
pull-request-automation
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
@getdave getdave deleted the fix/deprecate-use-api-fetch-hook branch Apr 21, 2020
@github-actions github-actions bot added this to the Gutenberg 8.0 milestone Apr 21, 2020
@getdave
Copy link
Contributor Author

@getdave 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
Reviewers

@marekhrabe marekhrabe

@aduth aduth

@retrofox retrofox

@youknowriad youknowriad

@noisysocks noisysocks

@tellthemachines tellthemachines

@mmtr mmtr

@nerrad nerrad

@ajitbohra ajitbohra

@ntwb ntwb

@talldan talldan

Assignees

@getdave getdave

Labels
[Package] API fetch [Package] Deprecated
Projects
None yet
Milestone
Gutenberg 8.0
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@getdave @aduth @marekhrabe