From 40f4b75d4f733660f788785726037731352a2785 Mon Sep 17 00:00:00 2001 From: Matyas Forian-Szabo Date: Wed, 11 Oct 2023 00:58:36 +0200 Subject: [PATCH 1/2] fix(ui-react-utils): fix ID counter wrong when not in context WIP, still needs unit tests --- .../DeterministicIdContext/DeterministicIdContext.ts | 5 +++-- .../DeterministicIdContextProvider.tsx | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContext.ts b/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContext.ts index 4ab89b0412..1d4b65eee4 100644 --- a/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContext.ts +++ b/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContext.ts @@ -24,6 +24,7 @@ import React from 'react' import { generateInstanceCounterMap } from './generateInstanceCounterMap' -const DeterministicIdContext = React.createContext(generateInstanceCounterMap()) +const defaultDeterministicIDMap = generateInstanceCounterMap() +const DeterministicIdContext = React.createContext(defaultDeterministicIDMap) -export { DeterministicIdContext } +export { DeterministicIdContext, defaultDeterministicIDMap } diff --git a/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx b/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx index 71e2f3b1f6..813d0fa1cf 100644 --- a/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx +++ b/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx @@ -22,15 +22,16 @@ * SOFTWARE. */ import React from 'react' -import { generateInstanceCounterMap } from './generateInstanceCounterMap' -import { DeterministicIdContext } from './DeterministicIdContext' +import { + DeterministicIdContext, + defaultDeterministicIDMap +} from './DeterministicIdContext' + type DeterministicIdProviderValue = Map type DeterministicIdProviderProps = React.PropsWithChildren<{ instanceCounterMap?: DeterministicIdProviderValue }> -const defaultContextValue = generateInstanceCounterMap() - /** * --- * category: components/utilities @@ -50,7 +51,7 @@ const DeterministicIdContextProvider = ({ ) } DeterministicIdContextProvider.defaultProps = { - instanceCounterMap: defaultContextValue + instanceCounterMap: defaultDeterministicIDMap } export { DeterministicIdContextProvider } From 2d44452724aaaeb834b8e607d43c60384508a172 Mon Sep 17 00:00:00 2001 From: Bence Toppanto Date: Wed, 11 Oct 2023 14:52:05 +0200 Subject: [PATCH 2/2] test(ui-react-utils): add tests and warnings to DeterministicIdProvider util --- .../DeterministicIdContextProvider.tsx | 5 ++ .../__tests__/DeterministicIdContext.test.tsx | 71 ++++++++++--------- 2 files changed, 41 insertions(+), 35 deletions(-) diff --git a/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx b/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx index 813d0fa1cf..bf37f0b3c9 100644 --- a/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx +++ b/packages/ui-react-utils/src/DeterministicIdContext/DeterministicIdContextProvider.tsx @@ -36,6 +36,11 @@ type DeterministicIdProviderProps = React.PropsWithChildren<{ * --- * category: components/utilities * --- + * WARNING: providing the `instanceCounterMap` prop will result in unexpected behaviour. DO NOT USE IT! + * + * DEPRECATED: the `instanceCounterMap` prop is deprecated. You don't need to supply the + * `instanceCounterMap` to the component. It handles it internally. + * * This is utility component for wrapping components with `DeterministicIdContext.Provider` * See detailed documentation about how to use it: [InstUISettingsProvider](/#InstUISettingsProvider) */ diff --git a/packages/ui-react-utils/src/__tests__/DeterministicIdContext.test.tsx b/packages/ui-react-utils/src/__tests__/DeterministicIdContext.test.tsx index bb9ead3aef..cfae13ba74 100644 --- a/packages/ui-react-utils/src/__tests__/DeterministicIdContext.test.tsx +++ b/packages/ui-react-utils/src/__tests__/DeterministicIdContext.test.tsx @@ -41,15 +41,45 @@ class TestComponent extends React.Component< } } +const uniqueIds = (el: { getDOMNode: () => Element }) => { + const idList = Array.from(el.getDOMNode().children).map((el) => el.id) + + return new Set(idList).size === idList.length +} + describe('DeterministicIdContext', () => { - it('should add id correctly by default', async () => { - const el = await mount() - const domNode = el.getDOMNode() + it('should generate unique ids without Provider wrapper', async () => { + const el = await mount( +
+ + + + + +
+ ) - expect(domNode.id).to.eq('TestComponent_0') + expect(uniqueIds(el)).to.be.true() }) + + it('should generate unique ids when components are rendered both out and inside of provider', async () => { + const el = await mount( +
+ + + + + + + +
+ ) + + expect(uniqueIds(el)).to.be.true() + }) + //skipping this test because it will fail either in strictmode or normal mode - it.skip('should increment id by default', async () => { + it('should generate unique ids with provider only', async () => { const Wrapper = ({ children }: any) => { return ( { } const el = await mount({children}) - Array.from(el.getDOMNode().children).forEach((el, i) => { - // since the double mounting we have to increase i by i*2 every iteration - expect(el.id).to.eq(`TestComponent_${i * 2}`) - }) - }) - - it('should work when instanceCounterMap is reset', async () => { - for (let i = 0; i < 10; i++) { - const el = await mount( - - - - ) - const domNode = el.getDOMNode() - - expect(domNode.id).to.eq('TestComponent_0') - } - }) - it('should work correctly when seeding the instanceCounterMap to the Context', async () => { - const seed = generateInstanceCounterMap() - seed.set('TestComponent', 20) - const el = await mount( - - - - ) - const domNode = el.getDOMNode() - expect(domNode.id).to.eq('TestComponent_21') + expect(uniqueIds(el)).to.be.true() }) })