From 4320d251dd224bd2a6a01de687c58acf2d39a2d4 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Fri, 5 Aug 2022 17:01:39 +0000 Subject: [PATCH 1/4] Add basic tests for `createSlots` --- src/__tests__/utils/create-slots.test.tsx | 56 +++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/__tests__/utils/create-slots.test.tsx diff --git a/src/__tests__/utils/create-slots.test.tsx b/src/__tests__/utils/create-slots.test.tsx new file mode 100644 index 00000000000..93bd5673f22 --- /dev/null +++ b/src/__tests__/utils/create-slots.test.tsx @@ -0,0 +1,56 @@ +import React from 'react' +import {render} from '@testing-library/react' +import createSlots from '../../utils/create-slots' + +describe('createSlots', () => { + it('moves slot contents to insertion point', () => { + const {Slots, Slot} = createSlots(['SlotName']) + + const Component = ({children}: {children: React.ReactNode}) => ( + + {slots => ( + <> +
{children}
+
{slots.SlotName}
+ + )} +
+ ) + + const {getByRole} = render( + + Slot ContentsOther Contents + + ) + + expect(getByRole('region', {name: 'children'})).toHaveTextContent('Other Contents') + expect(getByRole('region', {name: 'insertion point'})).toHaveTextContent('Slot Contents') + expect(getByRole('region', {name: 'children'})).not.toHaveTextContent('Slot Contents') + }) + + it('works with multiple slots', () => { + const {Slots, Slot} = createSlots(['A', 'B']) + + const Component = ({children}: {children: React.ReactNode}) => ( + + {slots => ( + <> +
{children}
+
{slots.A}
+
{slots.B}
+ + )} +
+ ) + + const {getByRole} = render( + + Slot A ContentsOther ContentsSlot B Contents + + ) + + expect(getByRole('region', {name: 'children'})).toHaveTextContent('Other Contents') + expect(getByRole('region', {name: 'a'})).toHaveTextContent('Slot A Contents') + expect(getByRole('region', {name: 'b'})).toHaveTextContent('Slot B Contents') + }) +}) From cd55092cb6936c6638b3650de69c2420ffbacb0d Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Fri, 5 Aug 2022 17:01:51 +0000 Subject: [PATCH 2/4] Fix infinite slot rendering when context prop not provided --- src/utils/create-slots.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/utils/create-slots.tsx b/src/utils/create-slots.tsx index 2a1b51eec7c..de1c73ca31a 100644 --- a/src/utils/create-slots.tsx +++ b/src/utils/create-slots.tsx @@ -23,6 +23,9 @@ const createSlots = (slotNames: SlotNames[]) => { context: {} }) + // maintain a static reference to avoid infinite render loop + const defaultContext = Object.freeze({}) + /** Slots uses a Double render strategy inspired by [reach-ui/descendants](https://github.com/reach/reach-ui/tree/develop/packages/descendants) * Slot registers themself with the Slots parent. * When all the children have mounted = registered themselves in slot, @@ -33,7 +36,7 @@ const createSlots = (slotNames: SlotNames[]) => { context?: ContextProps['context'] children: (slots: Slots) => React.ReactNode }> - > = ({context = {}, children}) => { + > = ({context = defaultContext, children}) => { // initialise slots const slotsDefinition: Slots = {} slotNames.map(name => (slotsDefinition[name] = null)) From 1fceb45f30c6b2a089ddff0cc5544b67b0c0a871 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Fri, 5 Aug 2022 13:45:49 -0400 Subject: [PATCH 3/4] Create changelog --- .changeset/fix-slots-infinite-render.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fix-slots-infinite-render.md diff --git a/.changeset/fix-slots-infinite-render.md b/.changeset/fix-slots-infinite-render.md new file mode 100644 index 00000000000..1227156515c --- /dev/null +++ b/.changeset/fix-slots-infinite-render.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Fix slots infinite rendering when no `context` prop is provided From fbee1fc64921594389617737da68cfd5d6bd12e1 Mon Sep 17 00:00:00 2001 From: Ian Sanders Date: Fri, 5 Aug 2022 19:09:39 +0000 Subject: [PATCH 4/4] Update to use existing tests --- src/__tests__/utils/create-slots.test.tsx | 56 ----------------------- src/__tests__/utils/createSlots.test.tsx | 12 ++--- 2 files changed, 6 insertions(+), 62 deletions(-) delete mode 100644 src/__tests__/utils/create-slots.test.tsx diff --git a/src/__tests__/utils/create-slots.test.tsx b/src/__tests__/utils/create-slots.test.tsx deleted file mode 100644 index 93bd5673f22..00000000000 --- a/src/__tests__/utils/create-slots.test.tsx +++ /dev/null @@ -1,56 +0,0 @@ -import React from 'react' -import {render} from '@testing-library/react' -import createSlots from '../../utils/create-slots' - -describe('createSlots', () => { - it('moves slot contents to insertion point', () => { - const {Slots, Slot} = createSlots(['SlotName']) - - const Component = ({children}: {children: React.ReactNode}) => ( - - {slots => ( - <> -
{children}
-
{slots.SlotName}
- - )} -
- ) - - const {getByRole} = render( - - Slot ContentsOther Contents - - ) - - expect(getByRole('region', {name: 'children'})).toHaveTextContent('Other Contents') - expect(getByRole('region', {name: 'insertion point'})).toHaveTextContent('Slot Contents') - expect(getByRole('region', {name: 'children'})).not.toHaveTextContent('Slot Contents') - }) - - it('works with multiple slots', () => { - const {Slots, Slot} = createSlots(['A', 'B']) - - const Component = ({children}: {children: React.ReactNode}) => ( - - {slots => ( - <> -
{children}
-
{slots.A}
-
{slots.B}
- - )} -
- ) - - const {getByRole} = render( - - Slot A ContentsOther ContentsSlot B Contents - - ) - - expect(getByRole('region', {name: 'children'})).toHaveTextContent('Other Contents') - expect(getByRole('region', {name: 'a'})).toHaveTextContent('Slot A Contents') - expect(getByRole('region', {name: 'b'})).toHaveTextContent('Slot B Contents') - }) -}) diff --git a/src/__tests__/utils/createSlots.test.tsx b/src/__tests__/utils/createSlots.test.tsx index 980572508db..92cde9b4f30 100644 --- a/src/__tests__/utils/createSlots.test.tsx +++ b/src/__tests__/utils/createSlots.test.tsx @@ -5,11 +5,11 @@ import createSlots from '../../utils/create-slots' // setup a component with slots const {Slots, Slot} = createSlots(['One', 'Two', 'Three']) -type ContextTypes = {salutation?: string} +type Props = {context?: {salutation: string}} -const ComponentWithSlots: React.FC> = ({salutation, children}) => { +const ComponentWithSlots: React.FC> = ({context, children}) => { return ( - + {slots => (
{slots.One} @@ -25,9 +25,9 @@ const SlotItem1: React.FC> = ({children}) => > = ({children}) => {children} const SlotItem3: React.FC> = ({children}) => ( - {(context: ContextTypes) => ( + {(context: Props['context']) => ( <> - {context.salutation} {children} + {context?.salutation} {children} )} @@ -64,7 +64,7 @@ describe('ComponentWithSlots', () => { it('renders with context passed to children', async () => { const component = render( - + third free form