Add possibility to manage innerBlocks while migrating deprecated blocks. #5932
Merged
jorgefilipecosta merged 1 commit intomasterfrom Apr 10, 2018
Merged
Conversation
8edc1bb to
aa7fc43
Compare
fa54943 to
59efeb4
Compare
youknowriad
reviewed
Apr 4, 2018
| const deprecatedBlockAttributes = getBlockAttributes( deprecatedBlockType, innerHTML, attributes ); | ||
| const migratedBlockAttributes = deprecatedBlockType.migrate ? deprecatedBlockType.migrate( deprecatedBlockAttributes ) : deprecatedBlockAttributes; | ||
| const migratedBlockAttributesAndInnerBlocks = deprecatedBlockType.migrate ? | ||
| deprecatedBlockType.migrate( { attributes: deprecatedBlockAttributes, innerBlocks } ) : |
Contributor
There was a problem hiding this comment.
Funny that to avoid breaking changes, we're introducing another breaking change here :) (especially the return value)
Options here:
- We're ok with this, assuming people didn't use
migrateyet. - We're not ok and we try to support the old API. Here's how I would achieve it:
- Instead of passing an object, pass the
innerBlocksas a second argument. - If you want to update both attributes and innerBlocks, return an array with two elements, if you return just an object assume we update only the attributes.
- Instead of passing an object, pass the
Contributor
There was a problem hiding this comment.
I'm still forming an opinion, but leaning toward number 2. In practice, I doubt that migrate has been used a lot in the wild up till now, but that's entirely speculative (thus epistemically irresponsible? 😄), and I think it's bad karma to be changing a migration API at this stage.
59efeb4 to
991e56c
Compare
aa7fc43 to
291d97c
Compare
5789d23 to
da419d3
Compare
7059339 to
21545b9
Compare
da419d3 to
df69227
Compare
21545b9 to
38bcf8d
Compare
3 tasks
Member
Author
|
This PR was updated to be back compatible I followed the suggestion and now we receive two arguments and return an array with attributes and inner blocks or an object with just the attributes. |
youknowriad
reviewed
Apr 10, 2018
| const migratedBlockAttributesAndInnerBlocks = deprecatedBlockType.migrate && | ||
| deprecatedBlockType.migrate( deprecatedBlockAttributes, innerBlocks ); | ||
|
|
||
| if ( isArray( migratedBlockAttributesAndInnerBlocks ) ) { |
Contributor
There was a problem hiding this comment.
Minor: Can we simplify the logic here or is it too clever?
const [ migratedAttributes, migratedInnerBlocks = innerBlocks ] = castArray( migratedBlockAttributesAndInnerBlocks );
Member
Author
There was a problem hiding this comment.
I like this version castArray simplifies the logic, I updated to use it :)
df69227 to
e541d8c
Compare
5d5d9ad to
fdc77b1
Compare
Member
Author
|
Thank you for the reviews @youknowriad and @mcsf! |
fdc77b1 to
8fb5d8e
Compare
8fb5d8e to
8e46963
Compare
…ks. Added migration logic to migrate existing cover images into the new nested version. Before the migrate function received the existing attributes and returned the new attributes now, migrate receives an object with attributes and innerBlocks and returns another object with the migrated attributes & innerBlocks.
8e46963 to
58ea91d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR was extracted from #5452, to make reviewing easier as this ones just focus on changes to the blocks API.
Description
Before the migrate function received the existing attributes and returned the new attributes now, migrate receives an object with attributes and innerBlocks and returns another object with the migrated attributes & innerBlocks.
This allows, for example, to migrate an attribute to an innerBlock, or the contrary move an inner block to be an attribute.
These changes were used to migrate existing cover image blocks to the nested version.
How Has This Been Tested?
To test the cover image migration merge this changes in a temporary branch with the nesting in cover image PR.
Add a post with following code:
Open the post in the visual editor and verify no invalid block error appears and the cover image blocks and paragraphs were migrated.