From 128279fc4f91f9d04cb1287cdf1b5ce7a54acd21 Mon Sep 17 00:00:00 2001 From: blam Date: Sat, 12 Jun 2021 17:41:21 +0200 Subject: [PATCH] feat: finishing up some other use cases and re-writing some logic with some more tests Signed-off-by: blam --- packages/app/src/App.tsx | 2 +- .../app/src/components/catalog/EntityPage.tsx | 4 +- .../core-app-api/src/routing/collectors.tsx | 4 +- .../src/extensions/useElementFilter.test.tsx | 207 ++++++++++++------ .../src/extensions/useElementFilter.tsx | 8 +- packages/core-plugin-api/src/index.test.ts | 1 + 6 files changed, 158 insertions(+), 68 deletions(-) diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index 12cbe89f26..de11afeed4 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -114,7 +114,7 @@ const routes = ( path="/tech-radar" element={} /> - + } /> } /> diff --git a/packages/app/src/components/catalog/EntityPage.tsx b/packages/app/src/components/catalog/EntityPage.tsx index 61d62136de..41d2d6f8c2 100644 --- a/packages/app/src/components/catalog/EntityPage.tsx +++ b/packages/app/src/components/catalog/EntityPage.tsx @@ -283,7 +283,7 @@ const serviceEntityPage = ( {overviewContent} - + {cicdContent} @@ -306,7 +306,7 @@ const serviceEntityPage = ( - + diff --git a/packages/core-app-api/src/routing/collectors.tsx b/packages/core-app-api/src/routing/collectors.tsx index 31c1ab6361..b9ca45f54a 100644 --- a/packages/core-app-api/src/routing/collectors.tsx +++ b/packages/core-app-api/src/routing/collectors.tsx @@ -177,8 +177,8 @@ export const featureFlagCollector = createCollector( () => new Set(), (acc, node) => { if (node.type === FeatureFlagged) { - const { flag } = node.props as FeatureFlaggedProps; - acc.add(flag); + const props = node.props as FeatureFlaggedProps; + acc.add('with' in props ? props.with : props.without); } }, ); diff --git a/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx b/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx index d5c1b0befe..bb2bcb06a5 100644 --- a/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx +++ b/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx @@ -37,8 +37,11 @@ attachComponentData(InnerComponent, INNER_COMPONENT_KEY, { }); const MockComponent = (_props: { children: ReactNode }) => null; -const FeatureFlagComponent = (_props: { children: ReactNode; flag: string }) => - null; +const FeatureFlagComponent = (_props: { + children: ReactNode; + with?: string; + without?: string; +}) => null; attachComponentData(FeatureFlagComponent, 'core.featureFlagged', true); const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); const Wrapper = ({ children }: { children?: React.ReactNode }) => ( @@ -151,73 +154,155 @@ describe('useElementFilter', () => { }); }); - it('should not discover deeper than the feature gate if the feature flag is disabled', () => { - jest.spyOn(mockFeatureFlagsApi, 'isActive').mockImplementation(() => false); - const tree = ( - - - + describe('FeatureFlags', () => { + describe('with', () => { + it('should not discover deeper than the feature gate if the feature flag is disabled', () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => false); + const tree = ( + + + + + + + + + + + - - - - + + ); + + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements + .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) + .getElements(), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); + + expect(result.current.length).toBe(1); + expect(result.current[0].key).toContain('second'); + }); + + it('should discover components behind a feature flag if the flag is enabled', () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => true); + const tree = ( + + + + + + + + + + + - - - - - ); + + ); - const { result } = renderHook( - props => - useElementFilter(props.tree, elements => - elements - .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) - .getElements(), - ), - { - initialProps: { tree }, - wrapper: Wrapper, - }, - ); + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements + .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) + .getElements(), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); - expect(result.current.length).toBe(1); - expect(result.current[0].key).toContain('second'); - }); + expect(result.current.length).toBe(2); + }); + }); - it('should discover components behind a feature flag if the flag is enabled', () => { - jest.spyOn(mockFeatureFlagsApi, 'isActive').mockImplementation(() => true); - const tree = ( - - - + describe('without', () => { + it('should discover deeper than the feature gate if the feature flag is disabled', () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => false); + const tree = ( + + + + + + + + + + + - - - - + + ); + + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements + .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) + .getElements(), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); + + expect(result.current.length).toBe(2); + }); + + it('should not discover components behind a feature flag if the flag is enabled', () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => true); + const tree = ( + + + + + + + + + + + - - - - - ); + + ); - const { result } = renderHook( - props => - useElementFilter(props.tree, elements => - elements - .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) - .getElements(), - ), - { - initialProps: { tree }, - wrapper: Wrapper, - }, - ); + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements + .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) + .getElements(), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); - expect(result.current.length).toBe(2); + expect(result.current.length).toBe(1); + }); + }); }); it('should reject when strict mode is enabled with the correct string', () => { @@ -252,7 +337,7 @@ describe('useElementFilter', () => { <> <> - + diff --git a/packages/core-plugin-api/src/extensions/useElementFilter.tsx b/packages/core-plugin-api/src/extensions/useElementFilter.tsx index 9ed070fd6c..b33c23e422 100644 --- a/packages/core-plugin-api/src/extensions/useElementFilter.tsx +++ b/packages/core-plugin-api/src/extensions/useElementFilter.tsx @@ -45,8 +45,12 @@ function selectChildren( } if (getComponentData(node, 'core.featureFlagged')) { - const { flag } = node.props as { flag: string }; - if (featureFlagsApi.isActive(flag)) { + const props = node.props as { with: string } | { without: string }; + const isEnabled = + 'with' in props + ? featureFlagsApi.isActive(props.with) + : !featureFlagsApi.isActive(props.without); + if (isEnabled) { return selectChildren( node.props.children, featureFlagsApi, diff --git a/packages/core-plugin-api/src/index.test.ts b/packages/core-plugin-api/src/index.test.ts index 9414a6b730..5d29ead619 100644 --- a/packages/core-plugin-api/src/index.test.ts +++ b/packages/core-plugin-api/src/index.test.ts @@ -40,6 +40,7 @@ describe('index', () => { useApi: expect.any(Function), useApiHolder: expect.any(Function), useApp: expect.any(Function), + useElementFilter: expect.any(Function), useRouteRef: expect.any(Function), useRouteRefParams: expect.any(Function), withApis: expect.any(Function),