Skip to content

Commit

Permalink
Validation: Log debugging messages for invalid blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
@aduth
aduth committed Nov 16, 2017
1 parent e7aaa99 commit 0749426
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 32 deletions.
6 changes: 3 additions & 3 deletions blocks/api/test/validation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe( 'validation', () => {
expect( isEqual ).toBe( true );
} );

it( 'returns true if not same style', () => {
it( 'returns false if not same style', () => {
const isEqual = isEqualAttributesOfName.style(
'background-image: url( "https://wordpress.org/img.png" ); color: red;',
'color: red; font-size: 13px; background-image: url(\'https://wordpress.org/img.png\');'
Expand Down Expand Up @@ -392,7 +392,7 @@ describe( 'validation', () => {
} );

describe( 'isValidBlock()', () => {
it( 'returns false is block is not valid', () => {
it( 'returns false if block is not valid', () => {
registerBlockType( 'core/test-block', defaultBlockSettings );

expect( isValidBlock(
Expand All @@ -402,7 +402,7 @@ describe( 'validation', () => {
) ).toBe( false );
} );

it( 'returns false is error occurs while generating block save', () => {
it( 'returns false if error occurs while generating block save', () => {
registerBlockType( 'core/test-block', {
...defaultBlockSettings,
save() {
Expand Down
109 changes: 80 additions & 29 deletions blocks/api/validation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,32 @@ const MEANINGFUL_ATTRIBUTES = [
...ENUMERATED_ATTRIBUTES,
];

/**
* Object of logger functions, logging messages in a development environment.
*/
const log = ( () => {
/**
* Creates a logger with block validation prefix, only in development.
*
* @param {Function} logger Original logger function
* @return {Function} Augmented logger function
*/
function createLogger( logger ) {
if ( 'development' !== process.env.NODE_ENV ) {
return () => {};
}

return ( message, ...args ) => logger( 'Block validation: ' + message, ...args );
}

return {
/* eslint-disable no-console */
error: createLogger( console.error ),
warning: createLogger( console.warn ),
/* eslint-enable no-console */
};
} )();

/**
* Given a specified string, returns an array of strings split by consecutive
* whitespace, ignoring leading or trailing whitespace.
Expand Down Expand Up @@ -172,16 +198,22 @@ export function getMeaningfulAttributePairs( token ) {
* Returns true if two text tokens (with `chars` property) are equivalent, or
* false otherwise.
*
* @param {Object} a First token
* @param {Object} b Second token
* @return {Boolean} Whether two text tokens are equivalent
* @param {Object} actual Actual token
* @param {Object} expected Expected token
* @return {Boolean} Whether two text tokens are equivalent
*/
export function isEqualTextTokensWithCollapsedWhitespace( a, b ) {
export function isEqualTextTokensWithCollapsedWhitespace( actual, expected ) {
// This is an overly simplified whitespace comparison. The specification is
// more prescriptive of whitespace behavior in inline and block contexts.
//
// See: https://medium.com/@patrickbrosset/when-does-white-space-matter-in-html-b90e8a7cdd33
return isEqual( ...[ a.chars, b.chars ].map( getTextWithCollapsedWhitespace ) );
const isEquivalentText = isEqual( ...[ actual.chars, expected.chars ].map( getTextWithCollapsedWhitespace ) );

if ( ! isEquivalentText ) {
log.warning( 'Expected text `%s`, saw `%s`.', expected.chars, actual.chars );
}

return isEquivalentText;
}

/**
Expand Down Expand Up @@ -230,52 +262,56 @@ export function getStyleProperties( text ) {
* @type {Object}
*/
export const isEqualAttributesOfName = {
class: ( a, b ) => {
class: ( actual, expected ) => {
// Class matches if members are the same, even if out of order or
// superfluous whitespace between.
return ! xor( ...[ a, b ].map( getTextPiecesSplitOnWhitespace ) ).length;
return ! xor( ...[ actual, expected ].map( getTextPiecesSplitOnWhitespace ) ).length;
},
style: ( a, b ) => {
return isEqual( ...[ a, b ].map( getStyleProperties ) );
style: ( actual, expected ) => {
return isEqual( ...[ actual, expected ].map( getStyleProperties ) );
},
};

/**
* Given two sets of attribute tuples, returns true if the attribute sets are
* equivalent
*
* @param {Array[]} a First attributes tuples
* @param {Array[]} b Second attributes tuples
* @return {Boolean} Whether attributes are equivalent
* @param {Array[]} actual Actual attributes tuples
* @param {Array[]} expected Expected attributes tuples
* @return {Boolean} Whether attributes are equivalent
*/
export function isEqualTagAttributePairs( a, b ) {
export function isEqualTagAttributePairs( actual, expected ) {
// Attributes is tokenized as tuples. Their lengths should match. This also
// avoids us needing to check both attributes sets, since if A has any keys
// which do not exist in B, we know the sets to be different.
if ( a.length !== b.length ) {
if ( actual.length !== expected.length ) {
log.warning( 'Expected attributes %o, instead saw %o.', expected, actual );
return false;
}

// Convert tuples to object for ease of lookup
const [ aAttributes, bAttributes ] = [ a, b ].map( fromPairs );
const [ actualAttributes, expectedAttributes ] = [ actual, expected ].map( fromPairs );

for ( const name in aAttributes ) {
for ( const name in actualAttributes ) {
// As noted above, if missing member in B, assume different
if ( ! bAttributes.hasOwnProperty( name ) ) {
if ( ! expectedAttributes.hasOwnProperty( name ) ) {
log.warning( 'Encountered unexpected attribute `%s`.', name );
return false;
}

const aValue = aAttributes[ name ];
const bValue = bAttributes[ name ];
const actualValue = actualAttributes[ name ];
const expectedValue = expectedAttributes[ name ];

const isEqualAttributes = isEqualAttributesOfName[ name ];
if ( isEqualAttributes ) {
// Defer custom attribute equality handling
if ( ! isEqualAttributes( aValue, bValue ) ) {
if ( ! isEqualAttributes( actualValue, expectedValue ) ) {
log.warning( 'Expected attribute `%s` of value `%s`, saw `%s`.', name, expectedValue, actualValue );
return false;
}
} else if ( aValue !== bValue ) {
} else if ( actualValue !== expectedValue ) {
// Otherwise strict inequality should bail
log.warning( 'Expected attribute `%s` of value `%s`, saw `%s`.', name, expectedValue, actualValue );
return false;
}
}
Expand All @@ -289,13 +325,14 @@ export function isEqualTagAttributePairs( a, b ) {
* @type {Object}
*/
export const isEqualTokensOfType = {
StartTag: ( a, b ) => {
if ( a.tagName !== b.tagName ) {
StartTag: ( actual, expected ) => {
if ( actual.tagName !== expected.tagName ) {
log.warning( 'Expected tag name `%s`, instead saw `%s`.', actual.tagName, expected.tagName );
return false;
}

return isEqualTagAttributePairs(
...[ a, b ].map( getMeaningfulAttributePairs )
...[ actual, expected ].map( getMeaningfulAttributePairs )
);
},
Chars: isEqualTextTokensWithCollapsedWhitespace,
Expand Down Expand Up @@ -328,25 +365,27 @@ export function getNextNonWhitespaceToken( tokens ) {
* Returns true if there is given HTML strings are effectively equivalent, or
* false otherwise.
*
* @param {String} a First HTML string
* @param {String} b Second HTML string
* @param {String} actual Actual HTML string
* @param {String} expected Expected HTML string
* @return {Boolean} Whether HTML strings are equivalent
*/
export function isEquivalentHTML( a, b ) {
export function isEquivalentHTML( actual, expected ) {
// Tokenize input content and reserialized save content
const [ actualTokens, expectedTokens ] = [ a, b ].map( tokenize );
const [ actualTokens, expectedTokens ] = [ actual, expected ].map( tokenize );

let actualToken, expectedToken;
while ( ( actualToken = getNextNonWhitespaceToken( actualTokens ) ) ) {
expectedToken = getNextNonWhitespaceToken( expectedTokens );

// Inequal if exhausted all expected tokens
if ( ! expectedToken ) {
log.warning( 'Expected end of content, instead saw %o.', actualToken );
return false;
}

// Inequal if next non-whitespace token of each set are not same type
if ( actualToken.type !== expectedToken.type ) {
log.warning( 'Expected token of type `%s` (%o), instead saw `%s` (%o).', expectedToken.type, expectedToken, actualToken.type, actualToken );
return false;
}

Expand All @@ -361,6 +400,7 @@ export function isEquivalentHTML( a, b ) {
while ( ( expectedToken = getNextNonWhitespaceToken( expectedTokens ) ) ) {
// If any non-whitespace tokens remain in expected token set, this
// indicates inequality
log.warning( 'Expected %o, instead saw end of content.', expectedToken );
return false;
}

Expand All @@ -387,5 +427,16 @@ export function isValidBlock( innerHTML, blockType, attributes ) {
return false;
}

return isEquivalentHTML( innerHTML, saveContent );
const isValid = isEquivalentHTML( innerHTML, saveContent );
if ( ! isValid ) {
log.error(
'Block validation failed for `%s` (%o).\n\nExpected:\n\n%s\n\nActual:\n\n%s',
blockType.name,
blockType,
saveContent,
innerHTML
);
}

return isValid;
}

0 comments on commit 0749426

Please sign in to comment.