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

Editor: Fix the issue where statics for deprecated components were not hoisted #16152

Merged
merged 1 commit into from Jun 14, 2019

Conversation

@gziolo
Copy link
Member

commented Jun 13, 2019

Description

This issue was discovered in Jetpack plugin, see Automattic/jetpack#12673.

There are a few components which were moved to @wordpress/block-editor package which were marked as deprecated. Unfortunately, they no longer expose statics exposed in the original components. Example:

wp.blockEditor.InnerBlocks exposes also, wp.blockEditor.InnerBlocks.Content

There was only wp.editor.InnerBlocks exposed for backward compatibility which obviously breaks the code in some plugins. This PR fixes it by exposing also missed statics.

How has this been tested?

Open JS console and ensure that all statics are hoisted:

  • BlockControls.Slot
  • BlockFormatControls.Slot
  • InnerBlocks.Content
  • InspectorAdvancedControls.Slot
  • InspectorControls.Slot
  • RichText.Content
  • RichText.isEmpty

@gziolo gziolo requested review from talldan and youknowriad as code owners Jun 13, 2019

@@ -41,7 +41,6 @@ import {
RichText as RootRichText,
RichTextShortcut as RootRichTextShortcut,
RichTextToolbarButton as RootRichTextToolbarButton,
RichTextInserterItem as RootRichTextInserterItem,

This comment has been minimized.

Copy link
@gziolo

gziolo Jun 13, 2019

Author Member

There is no such component in @wordpress/block-editor ...

@aduth

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

Is it something to consider this project, so we don't need to risk human error in specifying individual static properties to hoist?

https://github.com/mridgway/hoist-non-react-statics

@gziolo

This comment has been minimized.

Copy link
Member Author

commented Jun 13, 2019

Is it something to consider this project, so we don't need to risk human error in specifying individual static properties to hoist?

I considered it but I decided that it isn't that much useful in this context because of that whitelisted statics hoisted need to have deprecation logic applied as well. The library you shared only hoists statics and leaves them unchanged.

@gziolo gziolo requested a review from WordPress/gutenberg-core Jun 13, 2019

@youknowriad
Copy link
Contributor

left a comment

We can do a quick 5.9.1 with this fix. It should be as easy as cherry-picking this into the release/5.9 branch, pushing into the repository and running ./bin/commander.js stable

@gziolo gziolo merged commit 289e2a1 into master Jun 14, 2019

1 of 12 checks passed

Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Filter merged Filter merged
Details
Travis CI - Pull Request Build Passed
Details

@gziolo gziolo deleted the fix/editor-statics-not-hoisted-deprecations branch Jun 14, 2019

kellymears pushed a commit to kellymears/laravel-mix-wp-blocks that referenced this pull request Jun 14, 2019

gziolo added a commit that referenced this pull request Jun 14, 2019

daniloercoli added a commit that referenced this pull request Jun 14, 2019

Merge branch 'master' of https://github.com/WordPress/gutenberg into …
…rnmobile/open-video-by-browser-for-android

* 'master' of https://github.com/WordPress/gutenberg: (34 commits)
  [RNMobile] Native mobile release v1.7.0 (#16156)
  Scripts: Document and simplify changing file entry and output of build scripts (#15982)
  Block library: Refactor Heading block to use class names for text align (#16035)
  Make calendar block resilient against editor module not being present (#16161)
  Bump plugin version to 5.9.1
  Editor: Fix the issue where statics for deprecated components were not hoisted (#16152)
  Build Tooling: Use "full" `npm install` for Build Artifacts Travis task (#16166)
  Scripts: Fix naming conventions for function containing CLI keyword (#16091)
  Add native support for Inserter (Ported from gutenberg-mobile) (#16114)
  docs(components/higher-order/with-spoken-messages): fix issue in example code (#16144)
  docs(components/with-focus-return): fix typo in README.md (#16143)
  docs(block-editor/components/inspector-controls): fix image path in README.md (#16145)
  Add mention of on Figma to CONTRIBUTING.md (#16140)
  Bring greater consistency to placeholder text for media blocks. (#16135)
  Add descriptive text and a link to embed documentation in embed blocks (#16101)
  Update toolbar-text.png (#16102)
  Updating changelogs for the Gutenberg 5.9 packages release
  chore(release): publish
  [RNMobile] Fix pasting text on Post Title (#16116)
  Bump plugin version to 5.9.0
  ...

# Conflicts:
#	packages/block-library/src/video/video-player.android.js

nicolad added a commit to nicolad/gutenberg that referenced this pull request Jun 15, 2019

jg314 added a commit to jg314/gutenberg that referenced this pull request Jul 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.