Block API: Add support for transforms prioritization #5701
Conversation
aduth
added
the
[Feature] Block API
There was a problem hiding this comment.
Nice. I find the reuse of hooks clever. Outside this PR, it would be interesting to have some perf tests that we could incorporate into the test suite to track improvements and regressions. Right now, we only have anecdotal evidence to judge the impact of something like these changes (no apparent regression on my end).
| * @return {Array} Block transforms for direction. | ||
| */ | ||
| export function getBlockTransforms( direction, blockName ) { | ||
| // When retrieving transforms for all block types, recurse into self. |
There was a problem hiding this comment.
| const transformation = findTransform( transformsFrom, ( transform ) => ( | ||
| transform.type === 'shortcode' && | ||
| some( castArray( transform.tag ), ( tag ) => shortcode.regexp( tag ).test( HTML ) ) | ||
| ) ); |
There was a problem hiding this comment.
Nice simplification, along with the reduction of the while into if + recursion.
| // yielding non-deterministic results. A proper solution could be to | ||
| // let the editor (or site owners) determine a default block handler of | ||
| // unknown shortcodes — see `setUnknownTypeHandlerName`. | ||
| shortcode, |
test/unit/setup-wp-aliases.js
Outdated
| @@ -1,7 +1,8 @@ | |||
| // Set up `wp.*` aliases. Handled by Webpack outside of the test build. | |||
| global.wp = { | |||
| shortcode: { | |||
| next: () => {}, | |||
| next() {}, | |||
| regexp() {}, | |||
There was a problem hiding this comment.
This needs proper stubbing for tests.
aduth
force-pushed
the
try/transform-priority
branch
from
2ec86c3 to
777f7b6
Compare
|
Thanks for the review @mcsf .
Just slightly more accurate stubbing; at least returning a RegExp. This is resolved in the rebased 777f7b6. Of course, the fact that the stubbed RegExp is enough to allow tests to pass is a good signal we have poor test coverage for this functionality. Longer-term, we'd want to extract |
|
Re: Performance, in fact I might suspect a net performance improvement from the changes here, merely by dropping all the |
Fixes #5620
This pull request seeks to add a prioritization mechanism to block transforms, addressing two current issues with reliance on block registration order (paragraph and shortcode transforms as fallback). From a block implementer perspective, this simply adds a new property
priorityto the transform object, which behaves nearly identical to hooks prioritization (the similarity is not coincidental, as it uses hooks under the hood as the basis). At a code level, it introduces two new functionsgetBlockTransformsandfindTransformwhich respectively return/normalize transforms and return a predicate-matching transform considering priority.There may be room for further optimization here (e.g. sorting at registration time, caching transforms) but after a few failed attempts, I landed on this as a more direct/simple implementation.
Testing instructions:
Verify that there are no regressions in transform behavior. Notably, this impacts: