diff --git a/.changeset/few-windows-whisper.md b/.changeset/few-windows-whisper.md new file mode 100644 index 0000000000..565e8798a1 --- /dev/null +++ b/.changeset/few-windows-whisper.md @@ -0,0 +1,8 @@ +--- +'@backstage/core-app-api': patch +'@backstage/core-plugin-api': patch +'@backstage/plugin-catalog': patch +'@backstage/plugin-scaffolder': patch +--- + +Adding `FeatureFlag` component and treating `FeatureFlags` as first class citizens to composability API diff --git a/packages/app/package.json b/packages/app/package.json index 13b90d1af5..9d22f6bf28 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -8,6 +8,8 @@ "@backstage/cli": "^0.7.1", "@backstage/core": "^0.7.13", "@backstage/integration-react": "^0.1.3", + "@backstage/core-app-api": "^0.1.2", + "@backstage/core-components": "^0.1.1", "@backstage/plugin-api-docs": "^0.5.0", "@backstage/plugin-badges": "^0.2.2", "@backstage/plugin-catalog": "^0.6.3", diff --git a/packages/app/src/App.tsx b/packages/app/src/App.tsx index b14f9a3163..cc65cd65b3 100644 --- a/packages/app/src/App.tsx +++ b/packages/app/src/App.tsx @@ -14,13 +14,12 @@ * limitations under the License. */ +import { createApp, FlatRoutes } from '@backstage/core-app-api'; import { AlertDisplay, - createApp, - FlatRoutes, OAuthRequestDialog, SignInPage, -} from '@backstage/core'; +} from '@backstage/core-components'; import { apiDocsPlugin, ApiExplorerPage } from '@backstage/plugin-api-docs'; import { CatalogEntityPage, @@ -65,6 +64,7 @@ const app = createApp({ // Custom icon example alert: AlarmIcon, }, + components: { SignInPage: props => { return ( @@ -116,6 +116,7 @@ const routes = ( /> } /> } /> + } /> } /> } /> diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index b85c9392d7..5aa709121a 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -234,6 +234,18 @@ export type ErrorBoundaryFallbackProps = { resetError: () => void; }; +// @public (undocumented) +export const FeatureFlagged: (props: FeatureFlaggedProps) => JSX.Element; + +// @public (undocumented) +export type FeatureFlaggedProps = { + children: ReactNode; +} & ({ + with: string; +} | { + without: string; +}); + // @public (undocumented) export const FlatRoutes: (props: FlatRoutesProps) => JSX.Element | null; diff --git a/packages/core-app-api/src/app/App.tsx b/packages/core-app-api/src/app/App.tsx index a5ba5c4bca..ecf06955b0 100644 --- a/packages/core-app-api/src/app/App.tsx +++ b/packages/core-app-api/src/app/App.tsx @@ -57,6 +57,7 @@ import { } from '../extensions/traversal'; import { pluginCollector } from '../plugins/collectors'; import { + featureFlagCollector, routeObjectCollector, routeParentCollector, routePathCollector, @@ -215,7 +216,12 @@ export class PrivateAppImpl implements BackstageApp { [], ); - const { routePaths, routeParents, routeObjects } = useMemo(() => { + const { + routePaths, + routeParents, + routeObjects, + featureFlags, + } = useMemo(() => { const result = traverseElementTree({ root: children, discoverers: [childDiscoverer, routeElementDiscoverer], @@ -224,6 +230,7 @@ export class PrivateAppImpl implements BackstageApp { routeParents: routeParentCollector, routeObjects: routeObjectCollector, collectedPlugins: pluginCollector, + featureFlags: featureFlagCollector, }, }); @@ -238,7 +245,6 @@ export class PrivateAppImpl implements BackstageApp { // Initialize APIs once all plugins are available this.getApiHolder(); - return result; }, [children]); @@ -273,8 +279,14 @@ export class PrivateAppImpl implements BackstageApp { } } } + + // Go through the featureFlags returned from the traversal and + // register those now the configApi has been loaded + for (const name of featureFlags) { + featureFlagsApi.registerFlag({ name, pluginId: '' }); + } } - }, [hasConfigApi, loadedConfig]); + }, [hasConfigApi, loadedConfig, featureFlags]); if ('node' in loadedConfig) { // Loading or error diff --git a/packages/core-app-api/src/index.test.ts b/packages/core-app-api/src/index.test.ts index bec176fda8..654c547fca 100644 --- a/packages/core-app-api/src/index.test.ts +++ b/packages/core-app-api/src/index.test.ts @@ -37,6 +37,7 @@ describe('index', () => { ConfigReader: expect.any(Function), ErrorAlerter: expect.any(Function), ErrorApiForwarder: expect.any(Function), + FeatureFlagged: expect.any(Function), GithubAuth: expect.any(Function), GitlabAuth: expect.any(Function), GoogleAuth: expect.any(Function), diff --git a/packages/core-app-api/src/routing/FeatureFlagged.test.tsx b/packages/core-app-api/src/routing/FeatureFlagged.test.tsx new file mode 100644 index 0000000000..e20af76ae1 --- /dev/null +++ b/packages/core-app-api/src/routing/FeatureFlagged.test.tsx @@ -0,0 +1,102 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { FeatureFlagged } from './FeatureFlagged'; +import { render } from '@testing-library/react'; +import { ApiProvider, ApiRegistry, LocalStorageFeatureFlags } from '../apis'; +import { featureFlagsApiRef } from '@backstage/core-plugin-api'; + +const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); +const Wrapper = ({ children }: { children?: React.ReactNode }) => ( + + {children} + +); + +describe('FeatureFlagged', () => { + describe('with', () => { + it('should render contents when the feature flag is enabled', async () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => true); + + const { queryByText } = render( + +
+ +

BACKSTAGE!

+ +
+ , + ); + + expect(await queryByText('BACKSTAGE!')).toBeInTheDocument(); + }); + it('should not render contents when the feature flag is disabled', async () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => false); + + const { queryByText } = render( + +
+ +

BACKSTAGE!

+ +
+ , + ); + + expect(await queryByText('BACKSTAGE!')).not.toBeInTheDocument(); + }); + }); + describe('without', () => { + it('should not render contents when the feature flag is enabled', async () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => true); + + const { queryByText } = render( + +
+ +

BACKSTAGE!

+ +
+ , + ); + + expect(await queryByText('BACKSTAGE!')).not.toBeInTheDocument(); + }); + it('should render contents when the feature flag is disabled', async () => { + jest + .spyOn(mockFeatureFlagsApi, 'isActive') + .mockImplementation(() => false); + + const { queryByText } = render( + +
+ +

BACKSTAGE!

+ +
+ , + ); + + expect(await queryByText('BACKSTAGE!')).toBeInTheDocument(); + }); + }); +}); diff --git a/packages/core-app-api/src/routing/FeatureFlagged.tsx b/packages/core-app-api/src/routing/FeatureFlagged.tsx new file mode 100644 index 0000000000..40ef445f92 --- /dev/null +++ b/packages/core-app-api/src/routing/FeatureFlagged.tsx @@ -0,0 +1,39 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + featureFlagsApiRef, + useApi, + attachComponentData, +} from '@backstage/core-plugin-api'; +import React, { ReactNode } from 'react'; + +export type FeatureFlaggedProps = { children: ReactNode } & ( + | { with: string } + | { without: string } +); + +export const FeatureFlagged = (props: FeatureFlaggedProps) => { + const { children } = props; + const featureFlagApi = useApi(featureFlagsApiRef); + const isEnabled = + 'with' in props + ? featureFlagApi.isActive(props.with) + : !featureFlagApi.isActive(props.without); + return <>{isEnabled ? children : null}; +}; + +attachComponentData(FeatureFlagged, 'core.featureFlagged', true); diff --git a/packages/core-app-api/src/routing/FlatRoutes.test.tsx b/packages/core-app-api/src/routing/FlatRoutes.test.tsx index a9b83d1016..3ed5d5eb3a 100644 --- a/packages/core-app-api/src/routing/FlatRoutes.test.tsx +++ b/packages/core-app-api/src/routing/FlatRoutes.test.tsx @@ -17,25 +17,36 @@ import { render, RenderResult } from '@testing-library/react'; import React, { ReactNode } from 'react'; import { MemoryRouter, Route, Routes, useOutlet } from 'react-router-dom'; +import { ApiProvider, ApiRegistry, LocalStorageFeatureFlags } from '../apis'; +import { featureFlagsApiRef } from '@backstage/core-plugin-api'; import { AppContext } from '../app'; import { AppContextProvider } from '../app/AppContext'; import { FlatRoutes } from './FlatRoutes'; +const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); +const Wrapper = ({ children }: { children?: React.ReactNode }) => ( + + {children} + +); + function makeRouteRenderer(node: ReactNode) { let rendered: RenderResult | undefined = undefined; return (path: string) => { const content = ( - ({ - NotFoundErrorPage: () => <>Not Found, - }), - } as unknown) as AppContext - } - > - - + + ({ + NotFoundErrorPage: () => <>Not Found, + }), + } as unknown) as AppContext + } + > + + + ); if (rendered) { rendered.unmount(); diff --git a/packages/core-app-api/src/routing/FlatRoutes.tsx b/packages/core-app-api/src/routing/FlatRoutes.tsx index 54ef092965..ad1f764e6c 100644 --- a/packages/core-app-api/src/routing/FlatRoutes.tsx +++ b/packages/core-app-api/src/routing/FlatRoutes.tsx @@ -14,53 +14,16 @@ * limitations under the License. */ -import React, { ReactNode, Children, isValidElement, Fragment } from 'react'; +import React, { ReactNode } from 'react'; import { useRoutes } from 'react-router-dom'; -import { useApp } from '@backstage/core-plugin-api'; +import { useApp, useElementFilter } from '@backstage/core-plugin-api'; type RouteObject = { path: string; - element: JSX.Element; + element: ReactNode; children?: RouteObject[]; }; -// Similar to the same function from react-router, this collects routes from the -// children, but only the first level of routes -function createRoutesFromChildren(childrenNode: ReactNode): RouteObject[] { - return Children.toArray(childrenNode).flatMap(child => { - if (!isValidElement(child)) { - return []; - } - - const { children } = child.props; - - if (child.type === Fragment) { - return createRoutesFromChildren(children); - } - - let path = child.props.path as string | undefined; - - // TODO(Rugvip): Work around plugins registering empty paths, remove once deprecated routes are gone - if (path === '') { - return []; - } - path = path?.replace(/\/\*$/, '') ?? '/'; - - return [ - { - path, - element: child, - children: children && [ - { - path: '/*', - element: children, - }, - ], - }, - ]; - }); -} - type FlatRoutesProps = { children: ReactNode; }; @@ -68,14 +31,41 @@ type FlatRoutesProps = { export const FlatRoutes = (props: FlatRoutesProps): JSX.Element | null => { const app = useApp(); const { NotFoundErrorPage } = app.getComponents(); - const routes = createRoutesFromChildren(props.children) - // Routes are sorted to work around a bug where prefixes are unexpectedly matched - .sort((a, b) => b.path.localeCompare(a.path)) - // We make sure all routes have '/*' appended, except '/' - .map(obj => { - obj.path = obj.path === '/' ? '/' : `${obj.path}/*`; - return obj; - }); + const routes = useElementFilter(props.children, elements => + elements + .getElements<{ path?: string; children: ReactNode }>() + .flatMap(child => { + let path = child.props.path; + + // TODO(Rugvip): Work around plugins registering empty paths, remove once deprecated routes are gone + if (path === '') { + return []; + } + path = path?.replace(/\/\*$/, '') ?? '/'; + + return [ + { + path, + element: child, + children: child.props.children + ? [ + { + path: '/*', + element: child.props.children, + }, + ] + : undefined, + }, + ]; + }) + // Routes are sorted to work around a bug where prefixes are unexpectedly matched + .sort((a, b) => b.path.localeCompare(a.path)) + // We make sure all routes have '/*' appended, except '/' + .map(obj => { + obj.path = obj.path === '/' ? '/' : `${obj.path}/*`; + return obj; + }), + ); // TODO(Rugvip): Possibly add a way to skip this, like a noNotFoundPage prop routes.push({ diff --git a/packages/core-app-api/src/routing/collectors.tsx b/packages/core-app-api/src/routing/collectors.tsx index e940a2cada..b9ca45f54a 100644 --- a/packages/core-app-api/src/routing/collectors.tsx +++ b/packages/core-app-api/src/routing/collectors.tsx @@ -19,6 +19,7 @@ import { RouteRef } from '@backstage/core-plugin-api'; import { BackstageRouteObject } from './types'; import { getComponentData } from '../extensions'; import { createCollector } from '../extensions/traversal'; +import { FeatureFlagged, FeatureFlaggedProps } from './FeatureFlagged'; function getMountPoint(node: ReactElement): RouteRef | undefined { const element: ReactNode = node.props?.element; @@ -171,3 +172,13 @@ export const routeObjectCollector = createCollector( return parentObj; }, ); + +export const featureFlagCollector = createCollector( + () => new Set(), + (acc, node) => { + if (node.type === FeatureFlagged) { + const props = node.props as FeatureFlaggedProps; + acc.add('with' in props ? props.with : props.without); + } + }, +); diff --git a/packages/core-app-api/src/routing/index.ts b/packages/core-app-api/src/routing/index.ts index 7982333f4b..b37b51e919 100644 --- a/packages/core-app-api/src/routing/index.ts +++ b/packages/core-app-api/src/routing/index.ts @@ -15,3 +15,5 @@ */ export { FlatRoutes } from './FlatRoutes'; +export { FeatureFlagged } from './FeatureFlagged'; +export type { FeatureFlaggedProps } from './FeatureFlagged'; diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index a6f196847a..90ea1f5e46 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -8,6 +8,7 @@ import { BackstageTheme } from '@backstage/theme'; import { ComponentType } from 'react'; import { Config } from '@backstage/config'; import { default as React_2 } from 'react'; +import { ReactElement } from 'react'; import { ReactNode } from 'react'; import { SvgIconProps } from '@material-ui/core'; @@ -227,6 +228,20 @@ export type DiscoveryApi = { // @public (undocumented) export const discoveryApiRef: ApiRef; +// @public +export interface ElementCollection { + findComponentData(query: { + key: string; + }): T[]; + getElements(): Array>; + selectByComponentData(query: { + key: string; + withStrictError?: string; + }): ElementCollection; +} + // @public export type ErrorApi = { post(error: Error_2, context?: ErrorContext): void; @@ -514,6 +529,9 @@ export function useApiHolder(): ApiHolder; // @public (undocumented) export const useApp: () => AppContext; +// @public +export function useElementFilter(node: ReactNode, filterFn: (arg: ElementCollection) => T, dependencies?: any[]): T; + // @public (undocumented) export type UserFlags = {}; diff --git a/packages/core-plugin-api/package.json b/packages/core-plugin-api/package.json index 816727ed9e..48706a5877 100644 --- a/packages/core-plugin-api/package.json +++ b/packages/core-plugin-api/package.json @@ -42,6 +42,7 @@ }, "devDependencies": { "@backstage/cli": "^0.7.0", + "@backstage/core-app-api": "^0.1.2", "@backstage/test-utils": "^0.1.13", "@backstage/test-utils-core": "^0.1.1", "@testing-library/jest-dom": "^5.10.1", diff --git a/packages/core-plugin-api/src/extensions/index.ts b/packages/core-plugin-api/src/extensions/index.ts index 26a0c597b1..0ee1447b4e 100644 --- a/packages/core-plugin-api/src/extensions/index.ts +++ b/packages/core-plugin-api/src/extensions/index.ts @@ -20,3 +20,5 @@ export { createRoutableExtension, createComponentExtension, } from './extensions'; +export { useElementFilter } from './useElementFilter'; +export type { ElementCollection } from './useElementFilter'; diff --git a/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx b/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx new file mode 100644 index 0000000000..73fbb9e5a3 --- /dev/null +++ b/packages/core-plugin-api/src/extensions/useElementFilter.test.tsx @@ -0,0 +1,377 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React, { ReactNode } from 'react'; +import { useElementFilter } from './useElementFilter'; +import { renderHook } from '@testing-library/react-hooks'; +import { attachComponentData } from './componentData'; +import { featureFlagsApiRef } from '../apis'; +import { + ApiProvider, + ApiRegistry, + LocalStorageFeatureFlags, +} from '@backstage/core-app-api'; + +const WRAPPING_COMPONENT_KEY = 'core.blob.testing'; +const INNER_COMPONENT_KEY = 'core.blob2.testing'; + +const WrappingComponent = (_props: { children: ReactNode }) => null; +attachComponentData(WrappingComponent, WRAPPING_COMPONENT_KEY, { + message: 'hey! im wrapping component data', +}); +const InnerComponent = () => null; +attachComponentData(InnerComponent, INNER_COMPONENT_KEY, { + message: 'hey! im the inner component', +}); +const MockComponent = (_props: { children: ReactNode }) => 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 }) => ( + + {children} + +); + +describe('useElementFilter', () => { + it('should select elements based on a component data key', () => { + 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); + expect(result.current[0].key).toBe('.$.$first'); + expect(result.current[1].key).toBe('.$.$second'); + }); + + it('should find componentData', () => { + const tree = ( + + + + + + + + + + + ); + + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements.findComponentData({ key: WRAPPING_COMPONENT_KEY }), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); + + expect(result.current.length).toBe(2); + expect(result.current[0]).toEqual({ + message: 'hey! im wrapping component data', + }); + expect(result.current[1]).toEqual({ + message: 'hey! im wrapping component data', + }); + }); + + it('can be combined to together to filter the selection', () => { + const tree = ( + + + + + + + + + + + + ); + + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements + .selectByComponentData({ key: WRAPPING_COMPONENT_KEY }) + .findComponentData({ key: INNER_COMPONENT_KEY }), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); + + expect(result.current.length).toBe(2); + expect(result.current[0]).toEqual({ + message: 'hey! im the inner component', + }); + expect(result.current[1]).toEqual({ + message: 'hey! im the inner component', + }); + }); + + 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, + }, + ); + + expect(result.current.length).toBe(2); + }); + }); + + 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, + }, + ); + + expect(result.current.length).toBe(1); + }); + }); + }); + + it('should reject when strict mode is enabled with the correct string', () => { + const tree = ( + +

Hello

+ + ); + + const { result } = renderHook( + props => + useElementFilter(props.tree, elements => + elements + .selectByComponentData({ + key: WRAPPING_COMPONENT_KEY, + withStrictError: 'Could not find component', + }) + .findComponentData({ key: INNER_COMPONENT_KEY }), + ), + { + initialProps: { tree }, + wrapper: Wrapper, + }, + ); + + expect(result.error.message).toEqual('Could not find component'); + }); + + it('should support fragments and text node iteration', () => { + jest.spyOn(mockFeatureFlagsApi, 'isActive').mockImplementation(() => true); + const tree = ( + <> + + <> + + + + + + + + hello my name + <> + + + + + + is text + + + + ); + + 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); + }); +}); diff --git a/packages/core-plugin-api/src/extensions/useElementFilter.tsx b/packages/core-plugin-api/src/extensions/useElementFilter.tsx new file mode 100644 index 0000000000..0549472ab2 --- /dev/null +++ b/packages/core-plugin-api/src/extensions/useElementFilter.tsx @@ -0,0 +1,188 @@ +/* + * Copyright 2021 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + Children, + Fragment, + isValidElement, + ReactNode, + ReactElement, + useMemo, +} from 'react'; +import { getComponentData } from './componentData'; +import { useApi, FeatureFlagsApi, featureFlagsApiRef } from '../apis'; + +function selectChildren( + rootNode: ReactNode, + featureFlagsApi: FeatureFlagsApi, + selector?: (element: ReactElement) => boolean, + strictError?: string, +): Array> { + return Children.toArray(rootNode).flatMap(node => { + if (!isValidElement(node)) { + return []; + } + + if (node.type === Fragment) { + return selectChildren( + node.props.children, + featureFlagsApi, + selector, + strictError, + ); + } + + if (getComponentData(node, 'core.featureFlagged')) { + 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, + selector, + strictError, + ); + } + return []; + } + + if (selector === undefined || selector(node)) { + return [node]; + } + + if (strictError) { + throw new Error(strictError); + } + + return selectChildren( + node.props.children, + featureFlagsApi, + selector, + strictError, + ); + }); +} + +/** + * A querying interface tailored to traversing a set of selected React elements + * and extracting data. + * + * Methods prefixed with `selectBy` are used to narrow the set of selected elements. + * + * Methods prefixed with `find` return concrete data using a deep traversal of the set. + * + * Methods prefixed with `get` return concrete data using a shallow traversal of the set. + */ +export interface ElementCollection { + /** + * Narrows the set of selected components by doing a deep traversal and + * only including those that have defined component data for the given `key`. + * + * Whether an element in the tree has component data set for the given key + * is determined by whether `getComponentData` returns undefined. + * + * The traversal does not continue deeper past elements that match the criteria, + * and it also includes the root children in the selection, meaning that if the, + * of all the currently selected elements contain data for the given key, this + * method is a no-op. + * + * If `withStrictError` is set, the resulting selection must be a full match, meaning + * there may be no elements that were excluded in the selection. If the selection + * is not a clean match, an error will be throw with `withStrictError` as the message. + */ + selectByComponentData(query: { + key: string; + withStrictError?: string; + }): ElementCollection; + + /** + * Finds all elements using the same criteria as `selectByComponentData`, but + * returns the actual component data of each of those elements instead. + */ + findComponentData(query: { key: string }): T[]; + + /** + * Returns all of the elements currently selected by this collection. + */ + getElements(): Array< + ReactElement + >; +} + +class Collection implements ElementCollection { + constructor( + private readonly node: ReactNode, + private readonly featureFlagsApi: FeatureFlagsApi, + ) {} + + selectByComponentData(query: { key: string; withStrictError?: string }) { + const selection = selectChildren( + this.node, + this.featureFlagsApi, + node => getComponentData(node, query.key) !== undefined, + query.withStrictError, + ); + return new Collection(selection, this.featureFlagsApi); + } + + findComponentData(query: { key: string }): T[] { + const selection = selectChildren( + this.node, + this.featureFlagsApi, + node => getComponentData(node, query.key) !== undefined, + ); + return selection + .map(node => getComponentData(node, query.key)) + .filter((data: T | undefined): data is T => data !== undefined); + } + + getElements(): Array< + ReactElement + > { + return selectChildren(this.node, this.featureFlagsApi) as Array< + ReactElement + >; + } +} + +/** + * useElementFilter is a utility that helps you narrow down and retrieve data + * from a React element tree, typically operating on the `children` property + * passed in to a component. A common use-case is to construct declarative APIs + * where a React component defines its behavior based on its children, such as + * the relationship between `Routes` and `Route` in `react-router`. + * + * The purpose of this hook is similar to `React.Children.map`, and it expands upon + * it to also handle traversal of fragments and Backstage specific things like the + * `FeatureFlagged` component. + * + * The return value of the hook is computed by the provided filter function, but + * with added memoization based on the input `node`. If further memoization + * dependencies are used in the filter function, they should be added to the + * third `dependencies` argument, just like `useMemo`, `useEffect`, etc. + */ +export function useElementFilter( + node: ReactNode, + filterFn: (arg: ElementCollection) => T, + dependencies: any[] = [], +) { + const featureFlagsApi = useApi(featureFlagsApiRef); + const elements = new Collection(node, featureFlagsApi); + // eslint-disable-next-line react-hooks/exhaustive-deps + return useMemo(() => filterFn(elements), [node, ...dependencies]); +} 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), diff --git a/plugins/catalog/package.json b/plugins/catalog/package.json index 4e35cc7618..0af476b95a 100644 --- a/plugins/catalog/package.json +++ b/plugins/catalog/package.json @@ -33,6 +33,7 @@ "@backstage/catalog-client": "^0.3.13", "@backstage/catalog-model": "^0.8.3", "@backstage/core": "^0.7.13", + "@backstage/core-plugin-api": "^0.1.2", "@backstage/errors": "^0.1.1", "@backstage/integration": "^0.5.6", "@backstage/integration-react": "^0.1.3", @@ -54,6 +55,7 @@ }, "devDependencies": { "@backstage/cli": "^0.7.1", + "@backstage/core-app-api": "^0.1.2", "@backstage/dev-utils": "^0.1.17", "@backstage/test-utils": "^0.1.13", "@testing-library/jest-dom": "^5.10.1", diff --git a/plugins/catalog/src/components/EntityLayout/EntityLayout.tsx b/plugins/catalog/src/components/EntityLayout/EntityLayout.tsx index dfd27631f8..aceb1da6f2 100644 --- a/plugins/catalog/src/components/EntityLayout/EntityLayout.tsx +++ b/plugins/catalog/src/components/EntityLayout/EntityLayout.tsx @@ -29,6 +29,7 @@ import { Progress, RoutedTabs, } from '@backstage/core'; +import { useElementFilter } from '@backstage/core-plugin-api'; import { EntityContext, EntityRefLinks, @@ -37,14 +38,7 @@ import { } from '@backstage/plugin-catalog-react'; import { Box, TabProps } from '@material-ui/core'; import { Alert } from '@material-ui/lab'; -import { - Children, - default as React, - Fragment, - isValidElement, - useContext, - useState, -} from 'react'; +import React, { useContext, useState } from 'react'; import { useNavigate } from 'react-router'; import { EntityContextMenu } from '../EntityContextMenu/EntityContextMenu'; import { FavouriteEntity } from '../FavouriteEntity/FavouriteEntity'; @@ -58,46 +52,14 @@ type SubRoute = { tabProps?: TabProps; }; +const dataKey = 'plugin.catalog.entityLayoutRoute'; + const Route: (props: SubRoute) => null = () => null; +attachComponentData(Route, dataKey, true); // This causes all mount points that are discovered within this route to use the path of the route itself attachComponentData(Route, 'core.gatherMountPoints', true); -function createSubRoutesFromChildren( - childrenProps: React.ReactNode, - entity: Entity | undefined, -): SubRoute[] { - // Directly comparing child.type with Route will not work with in - // combination with react-hot-loader in storybook - // https://github.com/gaearon/react-hot-loader/issues/304 - const routeType = ( - -
- - ).type; - - return Children.toArray(childrenProps).flatMap(child => { - if (!isValidElement(child)) { - return []; - } - - if (child.type === Fragment) { - return createSubRoutesFromChildren(child.props.children, entity); - } - - if (child.type !== routeType) { - throw new Error('Child of EntityLayout must be an EntityLayout.Route'); - } - - const { path, title, children, if: condition, tabProps } = child.props; - if (condition && entity && !condition(entity)) { - return []; - } - - return [{ path, title, children, tabProps }]; - }); -} - const EntityLayoutTitle = ({ entity, title, @@ -195,7 +157,29 @@ export const EntityLayout = ({ const { kind, namespace, name } = useEntityCompoundName(); const { entity, loading, error } = useContext(EntityContext); - const routes = createSubRoutesFromChildren(children, entity); + const routes = useElementFilter(children, elements => + elements + .selectByComponentData({ + key: dataKey, + withStrictError: 'Child of EntityLayout must be an EntityLayout.Route', + }) + .getElements() // all nodes, element data, maintain structure or not? + .flatMap(({ props }) => { + if (props.if && entity && !props.if(entity)) { + return []; + } + + return [ + { + path: props.path, + title: props.title, + children: props.children, + tabProps: props.tabProps, + }, + ]; + }), + ); + const { headerTitle, headerType } = headerProps( kind, namespace, diff --git a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx index 20b9aaba4f..5b21266d44 100644 --- a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx +++ b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.test.tsx @@ -20,6 +20,19 @@ import { render } from '@testing-library/react'; import React from 'react'; import { isKind } from './conditions'; import { EntitySwitch } from './EntitySwitch'; +import { featureFlagsApiRef } from '@backstage/core-plugin-api'; +import { + LocalStorageFeatureFlags, + ApiProvider, + ApiRegistry, +} from '@backstage/core-app-api'; + +const mockFeatureFlagsApi = new LocalStorageFeatureFlags(); +const Wrapper = ({ children }: { children?: React.ReactNode }) => ( + + {children} + +); describe('EntitySwitch', () => { it('should switch child when entity switches', () => { @@ -32,15 +45,17 @@ describe('EntitySwitch', () => { ); const rendered = render( - - {content} - , + + + {content} + + , ); expect(rendered.queryByText('A')).toBeInTheDocument(); @@ -48,15 +63,17 @@ describe('EntitySwitch', () => { expect(rendered.queryByText('C')).not.toBeInTheDocument(); rendered.rerender( - - {content} - , + + + {content} + + , ); expect(rendered.queryByText('A')).not.toBeInTheDocument(); @@ -64,15 +81,17 @@ describe('EntitySwitch', () => { expect(rendered.queryByText('C')).not.toBeInTheDocument(); rendered.rerender( - - {content} - , + + + {content} + + , ); expect(rendered.queryByText('A')).not.toBeInTheDocument(); @@ -88,24 +107,28 @@ describe('EntitySwitch', () => { }; const rendered = render( - - - - - - , + + + + + + + + , ); expect(rendered.queryByText('A')).toBeInTheDocument(); expect(rendered.queryByText('B')).not.toBeInTheDocument(); rendered.rerender( - - - - - - , + + + + + + + + , ); expect(rendered.queryByText('A')).not.toBeInTheDocument(); diff --git a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx index a3ede42288..4b12a7249b 100644 --- a/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx +++ b/plugins/catalog/src/components/EntitySwitch/EntitySwitch.tsx @@ -16,49 +16,40 @@ import { Entity } from '@backstage/catalog-model'; import { useEntity } from '@backstage/plugin-catalog-react'; +import { PropsWithChildren, ReactNode } from 'react'; import { - Children, - Fragment, - isValidElement, - PropsWithChildren, - ReactNode, - useMemo, -} from 'react'; + attachComponentData, + useElementFilter, +} from '@backstage/core-plugin-api'; + +const ENTITY_SWITCH_KEY = 'core.backstage.entitySwitch'; const EntitySwitchCase = (_: { if?: (entity: Entity) => boolean; children: ReactNode; }) => null; +attachComponentData(EntitySwitchCase, ENTITY_SWITCH_KEY, true); + type SwitchCase = { if?: (entity: Entity) => boolean; children: JSX.Element; }; -function createSwitchCasesFromChildren(childrenNode: ReactNode): SwitchCase[] { - return Children.toArray(childrenNode).flatMap(child => { - if (!isValidElement(child)) { - return []; - } - - if (child.type === Fragment) { - return createSwitchCasesFromChildren(child.props.children); - } - - if (child.type !== EntitySwitchCase) { - throw new Error(`Child of EntitySwitch is not an EntitySwitch.Case`); - } - - const { if: condition, children } = child.props; - return [{ if: condition, children }]; - }); -} - export const EntitySwitch = ({ children }: PropsWithChildren<{}>) => { const { entity } = useEntity(); - const switchCases = useMemo(() => createSwitchCasesFromChildren(children), [ - children, - ]); + const switchCases = useElementFilter(children, collection => + collection + .selectByComponentData({ + key: ENTITY_SWITCH_KEY, + withStrictError: 'Child of EntitySwitch is not an EntitySwitch.Case', + }) + .getElements() + .flatMap((element: React.ReactElement) => { + const { if: condition, children: elementsChildren } = element.props; + return [{ if: condition, children: elementsChildren }]; + }), + ); const matchingCase = switchCases.find(switchCase => switchCase.if ? switchCase.if(entity) : true, diff --git a/plugins/scaffolder/package.json b/plugins/scaffolder/package.json index 69d71e7966..120cd7e297 100644 --- a/plugins/scaffolder/package.json +++ b/plugins/scaffolder/package.json @@ -33,11 +33,12 @@ "@backstage/catalog-client": "^0.3.13", "@backstage/catalog-model": "^0.8.2", "@backstage/config": "^0.1.5", - "@backstage/errors": "^0.1.1", "@backstage/core": "^0.7.12", "@backstage/integration": "^0.5.6", "@backstage/integration-react": "^0.1.3", "@backstage/plugin-catalog-react": "^0.2.2", + "@backstage/core-plugin-api": "^0.1.2", + "@backstage/errors": "^0.1.1", "@backstage/theme": "^0.2.8", "@material-ui/core": "^4.11.0", "@material-ui/icons": "^4.9.1", @@ -46,10 +47,10 @@ "@rjsf/material-ui": "^2.4.0", "@types/react": "^16.9", "classnames": "^2.2.6", - "json-schema": "^0.3.0", "git-url-parse": "^11.4.4", "humanize-duration": "^3.25.1", "immer": "^9.0.1", + "json-schema": "^0.3.0", "luxon": "^1.25.0", "react": "^16.13.1", "react-dom": "^16.13.1", @@ -66,9 +67,9 @@ "@backstage/test-utils": "^0.1.13", "@testing-library/jest-dom": "^5.10.1", "@testing-library/react": "^11.2.5", + "@testing-library/react-hooks": "^3.3.0", "@testing-library/user-event": "^13.1.8", "@types/humanize-duration": "^3.18.1", - "@testing-library/react-hooks": "^3.3.0", "@types/jest": "^26.0.7", "@types/node": "^14.14.32", "cross-fetch": "^3.0.6", diff --git a/plugins/scaffolder/src/components/Router.tsx b/plugins/scaffolder/src/components/Router.tsx index 5ac663c7ea..ef0dd4082e 100644 --- a/plugins/scaffolder/src/components/Router.tsx +++ b/plugins/scaffolder/src/components/Router.tsx @@ -14,7 +14,7 @@ * limitations under the License. */ -import React, { useMemo } from 'react'; +import React from 'react'; import { Routes, Route, useOutlet } from 'react-router'; import { ScaffolderPage } from './ScaffolderPage'; import { TemplatePage } from './TemplatePage'; @@ -27,21 +27,24 @@ import { FIELD_EXTENSION_KEY, DEFAULT_SCAFFOLDER_FIELD_EXTENSIONS, } from '../extensions'; -import { collectComponentData, collectChildren } from '../extensions/helpers'; +import { useElementFilter } from '@backstage/core-plugin-api'; export const Router = () => { const outlet = useOutlet(); - const fieldExtensions = useMemo(() => { - const registeredExtensions = collectComponentData( - collectChildren(outlet, FIELD_EXTENSION_WRAPPER_KEY).flat(), - FIELD_EXTENSION_KEY, - ); + const foundExtensions = useElementFilter(outlet, elements => + elements + .selectByComponentData({ + key: FIELD_EXTENSION_WRAPPER_KEY, + }) + .findComponentData({ + key: FIELD_EXTENSION_KEY, + }), + ); - return registeredExtensions.length - ? registeredExtensions - : DEFAULT_SCAFFOLDER_FIELD_EXTENSIONS; - }, [outlet]); + const fieldExtensions = foundExtensions.length + ? foundExtensions + : DEFAULT_SCAFFOLDER_FIELD_EXTENSIONS; return ( diff --git a/plugins/scaffolder/src/extensions/helpers.test.tsx b/plugins/scaffolder/src/extensions/helpers.test.tsx deleted file mode 100644 index ed3b74a210..0000000000 --- a/plugins/scaffolder/src/extensions/helpers.test.tsx +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import React from 'react'; -import { collectComponentData, collectChildren } from './helpers'; -import { attachComponentData } from '@backstage/core'; - -describe('Extension Helpers', () => { - const createElementWithComponentData = ({ - type, - data, - }: { - type: string; - data: any; - }) => { - const element: React.ComponentType = () => null; - attachComponentData(element, type, data); - return element; - }; - - describe('collectChildren', () => { - it('should return the children of the component which has the correct componentData flag', () => { - const SearchElement = createElementWithComponentData({ - type: 'find.me', - data: {}, - }); - - const DontCareAboutme = createElementWithComponentData({ - type: 'dont.find.me', - data: {}, - }); - - const child1 = ( -
- hello -
- ); - - const child2 = ( -
-

Hello2

-
- ); - - const testCase = ( -
- - {child1} - {child1} - - {child2} - -

Hello!

- {child1} - -
- ); - - const children = collectChildren(testCase, 'find.me'); - - expect(children).toEqual([[child1, child1], child2, child1]); - }); - }); - - describe('collectComponentData', () => { - it('should return the componentData for particular nodes', () => { - const componentData1 = { help: 'im something' }; - const componentData2 = { help: 'im something else' }; - - const FirstElement = createElementWithComponentData({ - type: 'find.me', - data: componentData1, - }); - - const SecondElement = createElementWithComponentData({ - type: 'dont.find.me', - data: componentData2, - }); - - const testCase = [ - , - , - , - , - ]; - const returnedData = collectComponentData(testCase, 'find.me'); - - expect(returnedData).toEqual([ - componentData1, - componentData1, - componentData1, - ]); - }); - }); -}); diff --git a/plugins/scaffolder/src/extensions/helpers.ts b/plugins/scaffolder/src/extensions/helpers.ts deleted file mode 100644 index 2eac9780e5..0000000000 --- a/plugins/scaffolder/src/extensions/helpers.ts +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright 2021 Spotify AB - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -import React from 'react'; -import { getComponentData } from '@backstage/core'; - -export const collectComponentData = ( - children: React.ReactNode, - componentDataKey: string, -) => { - const stack = [children]; - const found: T[] = []; - - while (stack.length) { - const current: React.ReactNode = stack.pop()!; - - React.Children.forEach(current, child => { - if (!React.isValidElement(child)) { - return; - } - - const data = getComponentData(child, componentDataKey); - if (data) { - found.push(data); - } - - if (child.props.children) { - stack.push(child.props.children); - } - }); - } - - return found; -}; - -export const collectChildren = ( - component: React.ReactNode, - componentDataKey: string, -) => { - const stack = [component]; - const found: React.ReactNode[] = []; - - while (stack.length) { - const current: React.ReactNode = stack.pop()!; - - React.Children.forEach(current, child => { - if (!React.isValidElement(child)) { - return; - } - - if (child.props.children) { - if (getComponentData(child, componentDataKey)) { - found.push(child.props.children); - } - stack.push(child.props.children); - } - }); - } - - return found; -}; diff --git a/yarn.lock b/yarn.lock index cdfa538e80..208ad9e952 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1394,6 +1394,7 @@ "@backstage/catalog-client" "^0.3.13" "@backstage/catalog-model" "^0.8.2" "@backstage/core" "^0.7.12" + "@backstage/core-plugin-api" "^0.1.2" "@backstage/errors" "^0.1.1" "@backstage/integration" "^0.5.6" "@backstage/integration-react" "^0.1.3"