diff --git a/packages/blocks/src/api/registration.js b/packages/blocks/src/api/registration.js index c0153a703785ba..b56b9d5667b57f 100644 --- a/packages/blocks/src/api/registration.js +++ b/packages/blocks/src/api/registration.js @@ -13,6 +13,7 @@ import { pick, pickBy, some, + castArray, } from 'lodash'; /** @@ -303,11 +304,50 @@ export function registerBlockType( name, settings ) { return; } + validateVariations( settings ); + dispatch( blocksStore ).addBlockTypes( settings ); return settings; } +function validateVariations( settings ) { + if ( ! Array.isArray( settings.variations ) ) { + return; + } + const attributeKeys = Object.keys( settings.attributes || {} ); + settings.variations.forEach( ( variation ) => { + if ( Array.isArray( variation.isActive ) ) { + const undefinedAttributes = variation.isActive.filter( + ( attribute ) => ! attributeKeys.includes( attribute ) + ); + if ( undefinedAttributes.length === 0 ) { + return; + } + + console.warn( + `The block "${ settings.name }"'s "${ + variation.name + }" variation "isActive" array contains attributes ("${ undefinedAttributes.join( + '", "' + ) }") that aren't defined in the block type. They will be ignored.` + ); + variation.isActive = variation.isActive.filter( ( attribute ) => + attributeKeys.includes( attribute ) + ); + } else if ( + ! isFunction( variation.isActive ) && + typeof variation.isActive !== 'undefined' && + variation.isActive !== null + ) { + console.warn( + `The block "${ settings.name }"'s "${ variation.name }" variation "isActive" must be either an array or a function.` + ); + delete variation.isActive; + } + } ); +} + /** * Registers a new block collection to group blocks in the same namespace in the inserter. * @@ -577,6 +617,8 @@ export const getBlockVariations = ( blockName, scope ) => { * @param {WPBlockVariation} variation Object describing a block variation. */ export const registerBlockVariation = ( blockName, variation ) => { + const blockType = getBlockType( blockName ); + validateVariations( { ...blockType, variations: castArray( variation ) } ); dispatch( blocksStore ).addBlockVariations( blockName, variation ); }; diff --git a/packages/blocks/src/api/test/registration.js b/packages/blocks/src/api/test/registration.js index 3ef1c4713f8149..3f0153076303c7 100644 --- a/packages/blocks/src/api/test/registration.js +++ b/packages/blocks/src/api/test/registration.js @@ -648,6 +648,60 @@ describe( 'blocks', () => { } ); } ); + it( "should warn and remove isActive if a variation's isActive is not an array or function", () => { + const blockType = { + save: noop, + category: 'text', + title: 'block title', + variations: [ + { + name: 'variation-1', + isActive: 'ignore this', + }, + ], + }; + registerBlockType( 'core/test-block-with-variations', blockType ); + expect( + getBlockType( 'core/test-block-with-variations' ).variations + ).toEqual( [ + { + name: 'variation-1', + }, + ] ); + expect( console ).toHaveWarnedWith( + 'The block "core/test-block-with-variations"\'s "variation-1" variation "isActive" must be either an array or a function.' + ); + } ); + + it( "should warn and remove extra attribute if a variation isActive's array contains attribute that is not defined in the block type", () => { + const blockType = { + save: noop, + category: 'text', + title: 'block title', + attributes: { + link: {}, + }, + variations: [ + { + name: 'variation-1', + isActive: [ 'link', 'text' ], + }, + ], + }; + registerBlockType( 'core/test-block-with-variations', blockType ); + expect( + getBlockType( 'core/test-block-with-variations' ).variations + ).toEqual( [ + { + name: 'variation-1', + isActive: [ 'link' ], + }, + ] ); + expect( console ).toHaveWarnedWith( + 'The block "core/test-block-with-variations"\'s "variation-1" variation "isActive" array contains attributes ("text") that aren\'t defined in the block type. They will be ignored.' + ); + } ); + describe( 'applyFilters', () => { afterEach( () => { removeAllFilters( 'blocks.registerBlockType' ); diff --git a/packages/blocks/src/store/selectors.js b/packages/blocks/src/store/selectors.js index 6bd8957fbe9d93..7c27e629dd3b3c 100644 --- a/packages/blocks/src/store/selectors.js +++ b/packages/blocks/src/store/selectors.js @@ -110,15 +110,11 @@ export function getActiveBlockVariation( state, blockName, attributes, scope ) { const match = variations?.find( ( variation ) => { if ( Array.isArray( variation.isActive ) ) { - const blockType = getBlockType( state, blockName ); - const attributeKeys = Object.keys( blockType.attributes || {} ); - return variation.isActive - .filter( ( attribute ) => attributeKeys.includes( attribute ) ) - .every( - ( attribute ) => - attributes[ attribute ] === - variation.attributes[ attribute ] - ); + return variation.isActive.every( + ( attribute ) => + attributes[ attribute ] === + variation.attributes[ attribute ] + ); } return variation.isActive?.( attributes, variation.attributes ); diff --git a/packages/blocks/src/store/test/selectors.js b/packages/blocks/src/store/test/selectors.js index 47b521f05bc30e..772d74e4e766b1 100644 --- a/packages/blocks/src/store/test/selectors.js +++ b/packages/blocks/src/store/test/selectors.js @@ -463,69 +463,6 @@ describe( 'selectors', () => { } ) ).toEqual( variations[ 2 ] ); } ); - it( 'should ignore attributes that are not defined in the block type', () => { - const variations = [ - { - name: 'variation-1', - attributes: { - firstTestAttribute: 1, - secondTestAttribute: 10, - undefinedTestAttribute: 100, - }, - isActive: [ - 'firstTestAttribute', - 'secondTestAttribute', - 'undefinedTestAttribute', - ], - }, - { - name: 'variation-2', - attributes: { - firstTestAttribute: 2, - secondTestAttribute: 20, - undefinedTestAttribute: 200, - }, - isActive: [ - 'firstTestAttribute', - 'secondTestAttribute', - 'undefinedTestAttribute', - ], - }, - ]; - - const state = createBlockVariationsStateWithTestBlockType( - variations - ); - - expect( - getActiveBlockVariation( state, blockName, { - firstTestAttribute: 1, - secondTestAttribute: 10, - undefinedTestAttribute: 100, - } ) - ).toEqual( variations[ 0 ] ); - expect( - getActiveBlockVariation( state, blockName, { - firstTestAttribute: 1, - secondTestAttribute: 10, - undefinedTestAttribute: 1234, - } ) - ).toEqual( variations[ 0 ] ); - expect( - getActiveBlockVariation( state, blockName, { - firstTestAttribute: 2, - secondTestAttribute: 20, - undefinedTestAttribute: 200, - } ) - ).toEqual( variations[ 1 ] ); - expect( - getActiveBlockVariation( state, blockName, { - firstTestAttribute: 2, - secondTestAttribute: 20, - undefinedTestAttribute: 2345, - } ) - ).toEqual( variations[ 1 ] ); - } ); } ); describe( 'getDefaultBlockVariation', () => { it( 'should return the default variation when set', () => {