From 6313fad188e25569ed6cb54035e99439fe910299 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Wed, 25 Mar 2020 14:39:31 -0400 Subject: [PATCH 1/6] Element: Fix serializer handling of multiple distinct contexts --- packages/element/README.md | 2 +- packages/element/src/serialize.js | 11 +++++++--- packages/element/src/test/serialize.js | 30 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/element/README.md b/packages/element/README.md index 1a51dea4046de9..da805d2c5363d8 100755 --- a/packages/element/README.md +++ b/packages/element/README.md @@ -313,7 +313,7 @@ Serializes a React element to string. _Parameters_ - _element_ `WPElement`: Element to serialize. -- _context_ `?Object`: Context object. +- _context_ `[Map]`: Context object. - _legacyContext_ `?Object`: Legacy context object. _Returns_ diff --git a/packages/element/src/serialize.js b/packages/element/src/serialize.js index d5ef4e02a7758e..c74534ec869426 100644 --- a/packages/element/src/serialize.js +++ b/packages/element/src/serialize.js @@ -347,7 +347,7 @@ function getNormalStylePropertyValue( property, value ) { * Serializes a React element to string. * * @param {WPElement} element Element to serialize. - * @param {?Object} context Context object. + * @param {Map=} context Context object. * @param {?Object} legacyContext Legacy context object. * * @return {string} Serialized element. @@ -411,11 +411,16 @@ export function renderElement( element, context, legacyContext = {} ) { switch ( type && type.$$typeof ) { case Provider.$$typeof: - return renderChildren( props.children, props.value, legacyContext ); + context = new Map( context ); + context.set( type, props.value ); + return renderChildren( props.children, context, legacyContext ); case Consumer.$$typeof: return renderElement( - props.children( context || type._currentValue ), + props.children( + ( context && context.get( type._context.Provider ) ) || + type._currentValue + ), context, legacyContext ); diff --git a/packages/element/src/test/serialize.js b/packages/element/src/test/serialize.js index d13cea967d41cc..f4523ad99d64b0 100644 --- a/packages/element/src/test/serialize.js +++ b/packages/element/src/test/serialize.js @@ -317,6 +317,36 @@ describe( 'renderElement()', () => { expect( result ).toBe( 'inner provided|outer provided' ); } ); + it( 'renders proper value through Context API when nested, distinct providers present', () => { + const { + Consumer: FirstConsumer, + Provider: FirstProvider, + } = createContext(); + const { + Consumer: SecondConsumer, + Provider: SecondProvider, + } = createContext(); + + const result = renderElement( + + + + { ( first ) => ( + <> + First: { first }, Second:{ ' ' } + + { ( second ) => second } + + + ) } + + + + ); + + expect( result ).toBe( 'First: First, Second: Second' ); + } ); + it( 'renders RawHTML as its unescaped children', () => { const result = renderElement( { '' } ); From 90dacf0585ae306a1c85a96dd94fb7bede9f863f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 26 Mar 2020 09:35:59 -0400 Subject: [PATCH 2/6] Element: Add non-mixed test case for nested context --- packages/element/src/test/serialize.js | 29 ++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/element/src/test/serialize.js b/packages/element/src/test/serialize.js index f4523ad99d64b0..59f0274045317e 100644 --- a/packages/element/src/test/serialize.js +++ b/packages/element/src/test/serialize.js @@ -327,6 +327,35 @@ describe( 'renderElement()', () => { Provider: SecondProvider, } = createContext(); + const result = renderElement( + + + { ( first ) => ( + + + { ( second ) => + `First: ${ first }, Second: ${ second }` + } + + + ) } + + + ); + + expect( result ).toBe( 'First: First, Second: Second' ); + } ); + + it( 'renders proper value through Context API when nested, distinct providers present (mixed order)', () => { + const { + Consumer: FirstConsumer, + Provider: FirstProvider, + } = createContext(); + const { + Consumer: SecondConsumer, + Provider: SecondProvider, + } = createContext(); + const result = renderElement( From 014e8840e069e87d7b0aaf530d7656cb40bd0fa5 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 26 Mar 2020 09:49:55 -0400 Subject: [PATCH 3/6] Element: Add testt case for rendering Context directly Not officially valid, yet technically supported in React's implementation. See: https://github.com/facebook/react/blob/a16b34974508cd23ce0844ad09a0e37a879d5591/packages/react-dom/src/server/ReactPartialRenderer.js#L1158-L1160 --- packages/element/src/test/serialize.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/element/src/test/serialize.js b/packages/element/src/test/serialize.js index 59f0274045317e..a3143345bb7140 100644 --- a/packages/element/src/test/serialize.js +++ b/packages/element/src/test/serialize.js @@ -317,6 +317,16 @@ describe( 'renderElement()', () => { expect( result ).toBe( 'inner provided|outer provided' ); } ); + it( 'renders Context gracefully', () => { + const Context = createContext( 'Default' ); + + const result = renderElement( + { ( value ) => value } + ); + + expect( result ).toBe( 'Default' ); + } ); + it( 'renders proper value through Context API when nested, distinct providers present', () => { const { Consumer: FirstConsumer, From 947a58f5374de7824a1f53d2be925b351b6b4c4e Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 26 Mar 2020 09:51:14 -0400 Subject: [PATCH 4/6] Element: Guard property access to context consumer `_context` Multiple references in React's codebase check whether this is assigned. See: https://github.com/facebook/react/blob/a16b34974508cd23ce0844ad09a0e37a879d5591/packages/react-dom/src/server/ReactPartialRenderer.js#L1157 It's not entirely clear the exact circumstances this may occur at this point in code. Ideally there would be a test case verifying failure without this guard. --- packages/element/src/serialize.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/element/src/serialize.js b/packages/element/src/serialize.js index c74534ec869426..0206e858627bfe 100644 --- a/packages/element/src/serialize.js +++ b/packages/element/src/serialize.js @@ -418,7 +418,9 @@ export function renderElement( element, context, legacyContext = {} ) { case Consumer.$$typeof: return renderElement( props.children( - ( context && context.get( type._context.Provider ) ) || + ( context && + type._context && + context.get( type._context.Provider ) ) || type._currentValue ), context, From c64b80afe91fa80d684d3ced53b08a1e265ce903 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 26 Mar 2020 12:06:02 -0400 Subject: [PATCH 5/6] Try common Map implementation --- package-lock.json | 8 ++++++++ packages/element/package.json | 1 + packages/element/src/serialize.js | 3 ++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index a4184598a1d896..3074804685e8e4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11037,9 +11037,17 @@ "requires": { "@babel/runtime": "^7.8.3", "@wordpress/escape-html": "file:packages/escape-html", + "core-js-pure": "^3.6.4", "lodash": "^4.17.15", "react": "^16.9.0", "react-dom": "^16.9.0" + }, + "dependencies": { + "core-js-pure": { + "version": "3.6.4", + "resolved": "https://registry.npmjs.org/core-js-pure/-/core-js-pure-3.6.4.tgz", + "integrity": "sha512-epIhRLkXdgv32xIUFaaAry2wdxZYBi6bgM7cB136dzzXXa+dFyRLTZeLUJxnd8ShrmyVXBub63n2NHo2JAt8Cw==" + } } }, "@wordpress/env": { diff --git a/packages/element/package.json b/packages/element/package.json index 24a1efe4ef6091..2f69438ba2825f 100644 --- a/packages/element/package.json +++ b/packages/element/package.json @@ -25,6 +25,7 @@ "dependencies": { "@babel/runtime": "^7.8.3", "@wordpress/escape-html": "file:../escape-html", + "core-js-pure": "^3.6.4", "lodash": "^4.17.15", "react": "^16.9.0", "react-dom": "^16.9.0" diff --git a/packages/element/src/serialize.js b/packages/element/src/serialize.js index 0206e858627bfe..4d4d2ea421adb8 100644 --- a/packages/element/src/serialize.js +++ b/packages/element/src/serialize.js @@ -36,6 +36,7 @@ import { kebabCase, isPlainObject, } from 'lodash'; +import CoreJSMap from 'core-js-pure/features/map'; /** * WordPress dependencies @@ -411,7 +412,7 @@ export function renderElement( element, context, legacyContext = {} ) { switch ( type && type.$$typeof ) { case Provider.$$typeof: - context = new Map( context ); + context = new CoreJSMap( context ); context.set( type, props.value ); return renderChildren( props.children, context, legacyContext ); From 621789b5e071135ac1dfff294f4ff0016586cf2a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 26 Mar 2020 16:29:19 -0400 Subject: [PATCH 6/6] Try avoiding explicit argument to constructor --- packages/element/src/serialize.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/element/src/serialize.js b/packages/element/src/serialize.js index 4d4d2ea421adb8..54e63f824366dd 100644 --- a/packages/element/src/serialize.js +++ b/packages/element/src/serialize.js @@ -412,7 +412,7 @@ export function renderElement( element, context, legacyContext = {} ) { switch ( type && type.$$typeof ) { case Provider.$$typeof: - context = new CoreJSMap( context ); + context = context ? new CoreJSMap( context ) : new CoreJSMap(); context.set( type, props.value ); return renderChildren( props.children, context, legacyContext );