Skip to content

Conversation

@johnwatkins0
Copy link
Contributor

This addresses this issue opened on the AMP plugin repo: ampproject/amp-wp#5534 Note that this issue completely breaks the block editor when both plugins are active, making the two plugins incompatible.

The AMP plugin uses the editor.MediaUpload filter expecting to filter a class component. This is because the original component from Gutenberg core is a class component:

https://github.com/WordPress/gutenberg/blob/95188199c8b8045322d7f75a2666d47ea6504ad2/packages/media-utils/src/components/media-upload/index.js#L227

The Cloudinary plugin, however, uses the same filter to return a function component. When Cloudinary's callback runs first, the subsequent callback is no longer able to extend the component as expected, causing an error that breaks the editor.

For reference, here's where the AMP plugin extends the original component, expecting a class: https://github.com/ampproject/amp-wp/blob/6ceca505b3be3a75f460a641f9bfb208c7bf67e6/assets/src/block-editor/components/with-media-library-notice.js#L32

@johnwatkins0 johnwatkins0 changed the title Return a class component from the editor.MediaUpload for compatibility with other plugins Return a class component from the editor.MediaUpload filter for compatibility with other plugins Oct 23, 2020
@DavidCramer DavidCramer changed the base branch from master to fix/amp-conflict October 24, 2020 04:45
Copy link
Contributor

@DavidCramer DavidCramer left a comment

Choose a reason for hiding this comment

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

Thanks @johnwatkins0

@DavidCramer DavidCramer merged commit 3b118cb into cloudinary:fix/amp-conflict Oct 24, 2020
DavidCramer pushed a commit that referenced this pull request Oct 24, 2020
Merging since this is a correction for the #247 which was a PR to the wrong branch. Approved that but merged to a new branch from master, then resolved conflicts.
@johnwatkins0 johnwatkins0 deleted the fix/amp-editor-image-component-conflict branch October 25, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants