-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Block API: Ensure backwards compatibility for block matchers #3519
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3519 +/- ##
=========================================
+ Coverage 35.04% 35.1% +0.05%
=========================================
Files 263 265 +2
Lines 6705 6735 +30
Branches 1220 1221 +1
=========================================
+ Hits 2350 2364 +14
- Misses 3677 3693 +16
Partials 678 678
Continue to review full report at Codecov.
|
blocks/api/source.js
Outdated
function warnAboutDeprecatedMatcher() { | ||
// eslint-disable-next-line no-console | ||
console.warn( | ||
'Attributes matchers are deprecated and they will be remove in future version of Gutenberg.' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "remove" -> "removed"
Grammar: "in future version" -> "in a future version"
blocks/api/source.js
Outdated
// eslint-disable-next-line no-console | ||
console.warn( | ||
'Attributes matchers are deprecated and they will be remove in future version of Gutenberg.' + | ||
'Please update your attributes definition https://wordpress.org/gutenberg/handbook/reference/attributes/' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Grammar: There should be a space between these two sentences, either prefixing this string or suffixing the previous line. (or separate arguments to console.warn
which is maybe not the best behavior to rely on)
@@ -156,6 +156,7 @@ class ButtonBlock extends Component { | |||
</span>, | |||
focus && ( | |||
<form | |||
key="form-link" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this change intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, this was a bug fix I wanted to publish separately and accidentally made it to this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can extract it to a separate PR but maybe not worth it
blocks/api/registration.js
Outdated
@@ -125,6 +125,17 @@ export function registerBlockType( name, settings ) { | |||
...settings, | |||
}; | |||
|
|||
// Resolve deprecated attributes | |||
settings.attributes = mapValues( settings.attributes, ( attribute ) => { | |||
if ( isFunction( attribute.source ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to return functions from the deprecated matcher? Why not just return the object, and skip this step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it won't match the current API.
it will produce: source: { source: 'text' }
which is not the correct attribute definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be implemented as a filter hook, which has the added advantage of being simpler to drop 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea
c75bbc9
to
4ec5a9a
Compare
68291f8
to
d8249a9
Compare
return settings; | ||
} | ||
|
||
export default function anchor( { addFilter } ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copypasta.
with #2854 it's no longer possible to use attribute matchers to define the block attributes. This PR adds a way to ensure backwards compatibility while warning about the "deprecated" status of these matchers.
Testing instructions
source.attr( 'myattribute' )
)