From 8a52949640039c1307bd282b8887c1cfb31f693f Mon Sep 17 00:00:00 2001 From: manajdov Date: Tue, 22 Jan 2019 18:24:16 +0100 Subject: [PATCH 1/7] -reverted changed for not passing props to react elements in the factories --- src/lib/factories.ts | 13 ++++++------ test/specs/lib/factories-test.tsx | 33 +++++++++++++++++++------------ 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/lib/factories.ts b/src/lib/factories.ts index e98570ba4e..27faf7e6ff 100644 --- a/src/lib/factories.ts +++ b/src/lib/factories.ts @@ -129,18 +129,16 @@ function createShorthandFromValue( ) } - // return value 'as is' if it a ReactElement - if (valIsReactElement) { - return value as React.ReactElement - } - // ---------------------------------------- // Build up props // ---------------------------------------- const { defaultProps = {} } = options // User's props - const usersProps = valIsPropsObject ? (value as Props) : {} + const usersProps = + (valIsReactElement && (value as React.ReactElement).props) || + (valIsPropsObject && (value as Props)) || + {} // Override props let { overrideProps } = options @@ -199,6 +197,9 @@ function createShorthandFromValue( return render(Component, props) } + // Clone ReactElements + if (valIsReactElement) return React.cloneElement(value as React.ReactElement, props) + // Create ReactElements from built up props if (valIsPrimitive || valIsPropsObject) return React.createElement(Component, props) diff --git a/test/specs/lib/factories-test.tsx b/test/specs/lib/factories-test.tsx index 08a20db3ef..96463a78fe 100644 --- a/test/specs/lib/factories-test.tsx +++ b/test/specs/lib/factories-test.tsx @@ -492,6 +492,16 @@ describe('factories', () => { testCreateShorthand({ overrideProps, value: testValue }, overrideProps()) }) + test("is called with the user's element's and default props", () => { + const defaultProps = { 'data-some': 'defaults' } + const overrideProps = jest.fn(() => ({})) + const userProps = { 'data-user': 'props' } + const value =
+ + shallow(getShorthand({ defaultProps, overrideProps, value })) + expect(overrideProps).toHaveBeenCalledWith({ ...defaultProps, ...userProps }) + }) + test("is called with the user's props object", () => { const defaultProps = { 'data-some': 'defaults' } const overrideProps = jest.fn(() => ({})) @@ -524,21 +534,18 @@ describe('factories', () => { describe('from an element', () => { itReturnsAValidElement(
) + itAppliesDefaultProps(
) itDoesNotIncludePropsFromMappedProp(
) + itMergesClassNames('element', 'user', { value:
}) itAppliesProps('element', { foo: 'foo' }, { value:
}) - - test('forwards original element "as is"', () => { - testCreateShorthand( - { - Component: 'p', - value: ( - - ), - defaultProps: { commonProp: 'default', defaultProp: true }, - overrideProps: { commonProp: 'override', overrideProp: true }, - }, - { commonProp: 'originalElement', originalElementProp: true }, - ) + itOverridesDefaultProps( + 'element', + { some: 'defaults', overridden: false }, + { some: 'defaults', overridden: true }, + { value:
}, + ) + itOverridesDefaultPropsWithFalseyProps('element', { + value:
, }) }) From f83d928f71d97c7fbe421be982e3ffbe2a0714a4 Mon Sep 17 00:00:00 2001 From: manajdov Date: Wed, 23 Jan 2019 12:00:05 +0100 Subject: [PATCH 2/7] -updated shorthand props documentation --- docs/src/views/ShorthandProps.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/docs/src/views/ShorthandProps.tsx b/docs/src/views/ShorthandProps.tsx index 469bb5a2dd..273607d53c 100644 --- a/docs/src/views/ShorthandProps.tsx +++ b/docs/src/views/ShorthandProps.tsx @@ -86,14 +86,11 @@ const ShorthandProps = props => (
There is a very important caveat here, though: whenever React Element is directly used as a - shorthand value, it becomes client's responsibility to handle some aspects that Stardust was - originally responsible for (such as {code('styles')} or {code('accessibility')}). + shorthand value, all props that Stardust has created for the slot's Component will be spread + here. This means, you may end up with invalid prop applied on HTML element. {' '} - This is because, in contrast to other forms of shorthand values, Stardust-evaluated props - cannot be safely passed to the element which type is, generally, doesn't allow Stardust to - make any prior assumptions about. Due to this limitation, you should strive to use other - options for shorthand values whenever is possible - for instance, this is how previous example - can be rewritten: + Due to this limitation, you should strive to use other options for shorthand values whenever + is possible - for instance, this is how previous example can be rewritten:
{codeExample([`