From 514a1dd6a82247a9c8a9383f3773e267732d0f7a Mon Sep 17 00:00:00 2001 From: Luis Felipe Zaguini Date: Tue, 14 Nov 2023 00:33:36 +0100 Subject: [PATCH 1/2] PostCSS style transformation: fail gracefully instead of throwing error --- .../src/utils/test/transform-styles.js | 22 +++++++++++ .../src/utils/transform-styles/index.js | 38 ++++++++++++------- 2 files changed, 47 insertions(+), 13 deletions(-) diff --git a/packages/block-editor/src/utils/test/transform-styles.js b/packages/block-editor/src/utils/test/transform-styles.js index f162a0b2f6048c..b00720fd2142c4 100644 --- a/packages/block-editor/src/utils/test/transform-styles.js +++ b/packages/block-editor/src/utils/test/transform-styles.js @@ -4,6 +4,28 @@ import transformStyles from '../transform-styles'; describe( 'transformStyles', () => { + it( 'should not throw error in case of invalid css', () => { + const run = () => + transformStyles( + [ + { + css: 'h1 { color: red; }', // valid CSS + }, + { + css: 'h1 { color: red;', // invalid CSS + }, + ], + '.my-namespace' + ); + + expect( run ).not.toThrow(); + + const [ validCSS, invalidCSS ] = run(); + + expect( validCSS ).toBe( '.my-namespace h1 { color: red; }' ); + expect( invalidCSS ).toBe( null ); + } ); + describe( 'selector wrap', () => { it( 'should wrap regular selectors', () => { const input = `h1 { color: red; }`; diff --git a/packages/block-editor/src/utils/transform-styles/index.js b/packages/block-editor/src/utils/transform-styles/index.js index 8f5e1702307a45..4c7300d331d02c 100644 --- a/packages/block-editor/src/utils/transform-styles/index.js +++ b/packages/block-editor/src/utils/transform-styles/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import postcss from 'postcss'; +import postcss, { CssSyntaxError } from 'postcss'; import wrap from 'postcss-prefixwrap'; import rebaseUrl from 'postcss-urlrebase'; @@ -19,18 +19,30 @@ import rebaseUrl from 'postcss-urlrebase'; */ const transformStyles = ( styles, wrapperSelector = '' ) => { return styles.map( ( { css, ignoredSelectors = [], baseURL } ) => { - return postcss( - [ - wrapperSelector && - wrap( wrapperSelector, { - ignoredSelectors: [ - ...ignoredSelectors, - wrapperSelector, - ], - } ), - baseURL && rebaseUrl( { rootUrl: baseURL } ), - ].filter( Boolean ) - ).process( css, {} ).css; // use sync PostCSS API + try { + return postcss( + [ + wrapperSelector && + wrap( wrapperSelector, { + ignoredSelectors: [ + ...ignoredSelectors, + wrapperSelector, + ], + } ), + baseURL && rebaseUrl( { rootUrl: baseURL } ), + ].filter( Boolean ) + ).process( css, {} ).css; // use sync PostCSS API + } catch ( error ) { + if ( ! ( error instanceof CssSyntaxError ) ) { + // eslint-disable-next-line no-console + console.warn( + 'wp.blockEditor.transformStyles Failed to transform CSS.', + error + ); + } + + return null; + } } ); }; From bf1015980eabcd9b9c7925c36bbb6bb1bc349fa6 Mon Sep 17 00:00:00 2001 From: Luis Felipe Zaguini Date: Wed, 22 Nov 2023 12:24:13 +0100 Subject: [PATCH 2/2] Display better warning messages --- .../src/utils/test/transform-styles.js | 63 +++++++++++++------ .../src/utils/transform-styles/index.js | 8 ++- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/packages/block-editor/src/utils/test/transform-styles.js b/packages/block-editor/src/utils/test/transform-styles.js index b00720fd2142c4..38eaa1947d2a8c 100644 --- a/packages/block-editor/src/utils/test/transform-styles.js +++ b/packages/block-editor/src/utils/test/transform-styles.js @@ -4,26 +4,53 @@ import transformStyles from '../transform-styles'; describe( 'transformStyles', () => { - it( 'should not throw error in case of invalid css', () => { - const run = () => - transformStyles( - [ - { - css: 'h1 { color: red; }', // valid CSS - }, - { - css: 'h1 { color: red;', // invalid CSS - }, - ], - '.my-namespace' - ); - - expect( run ).not.toThrow(); + describe( 'error handling', () => { + beforeEach( () => { + // Intentionally suppress the expected console errors and warnings to reduce + // noise in the test output. + jest.spyOn( console, 'warn' ).mockImplementation( jest.fn() ); + } ); - const [ validCSS, invalidCSS ] = run(); + it( 'should not throw error in case of invalid css', () => { + const run = () => + transformStyles( + [ + { + css: 'h1 { color: red;', // invalid CSS + }, + ], + '.my-namespace' + ); + + expect( run ).not.toThrow(); + expect( console ).toHaveWarned(); + } ); - expect( validCSS ).toBe( '.my-namespace h1 { color: red; }' ); - expect( invalidCSS ).toBe( null ); + it( 'should warn invalid css in the console', () => { + const run = () => + transformStyles( + [ + { + css: 'h1 { color: red; }', // valid CSS + }, + { + css: 'h1 { color: red;', // invalid CSS + }, + ], + '.my-namespace' + ); + + const [ validCSS, invalidCSS ] = run(); + + expect( validCSS ).toBe( '.my-namespace h1 { color: red; }' ); + expect( invalidCSS ).toBe( null ); + + expect( console ).toHaveWarnedWith( + 'wp.blockEditor.transformStyles Failed to transform CSS.', + ':1:1: Unclosed block\n> 1 | h1 { color: red;\n | ^' + // ^^^^ In PostCSS, a tab is equal four spaces + ); + } ); } ); describe( 'selector wrap', () => { diff --git a/packages/block-editor/src/utils/transform-styles/index.js b/packages/block-editor/src/utils/transform-styles/index.js index 4c7300d331d02c..4da6f6ce237950 100644 --- a/packages/block-editor/src/utils/transform-styles/index.js +++ b/packages/block-editor/src/utils/transform-styles/index.js @@ -33,7 +33,13 @@ const transformStyles = ( styles, wrapperSelector = '' ) => { ].filter( Boolean ) ).process( css, {} ).css; // use sync PostCSS API } catch ( error ) { - if ( ! ( error instanceof CssSyntaxError ) ) { + if ( error instanceof CssSyntaxError ) { + // eslint-disable-next-line no-console + console.warn( + 'wp.blockEditor.transformStyles Failed to transform CSS.', + error.message + '\n' + error.showSourceCode( false ) + ); + } else { // eslint-disable-next-line no-console console.warn( 'wp.blockEditor.transformStyles Failed to transform CSS.',