From 0bdeeb20cfc689949762cf623603c5ec744eb741 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 25 Nov 2020 11:40:36 +0100 Subject: [PATCH 01/10] core-api: add internal attachNodeData and getComponentData helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam --- .../core-api/src/lib/componentData.test.tsx | 62 +++++++++++++++++ packages/core-api/src/lib/componentData.tsx | 69 +++++++++++++++++++ 2 files changed, 131 insertions(+) create mode 100644 packages/core-api/src/lib/componentData.test.tsx create mode 100644 packages/core-api/src/lib/componentData.tsx diff --git a/packages/core-api/src/lib/componentData.test.tsx b/packages/core-api/src/lib/componentData.test.tsx new file mode 100644 index 0000000000..6c4abda40f --- /dev/null +++ b/packages/core-api/src/lib/componentData.test.tsx @@ -0,0 +1,62 @@ +/* + * Copyright 2020 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 { attachComponentData, getComponentData } from './componentData'; + +describe('elementData', () => { + it('should attach a single piece of data', () => { + const data = { foo: 'bar' }; + const Component = () => null; + attachComponentData(Component, 'my-data', data); + + const element = ; + expect(getComponentData(element, 'my-data')).toBe(data); + }); + + it('should attach several distinct pieces of data', () => { + const data1 = { foo: 'bar' }; + const data2 = { test: 'value' }; + const Component = () => null; + attachComponentData(Component, 'my-data', data1); + attachComponentData(Component, 'second', data2); + + const element = ; + expect(getComponentData(element, 'my-data')).toBe(data1); + expect(getComponentData(element, 'second')).toBe(data2); + }); + + it('returns undefined for missing data', () => { + const data = { foo: 'bar' }; + const Component1 = () => null; + const Component2 = () => null; + attachComponentData(Component2, 'my-data', data); + + const element1 = ; + const element2 = ; + expect(getComponentData(element1, 'missing')).toBeUndefined(); + expect(getComponentData(element2, 'missing')).toBeUndefined(); + }); + + it('should throw when attempting to overwrite data', () => { + const data = { foo: 'bar' }; + const MyComponent = () => null; + attachComponentData(MyComponent, 'my-data', data); + expect(() => attachComponentData(MyComponent, 'my-data', data)).toThrow( + 'Attempted to attach duplicate data "my-data" to component "MyComponent"', + ); + }); +}); diff --git a/packages/core-api/src/lib/componentData.tsx b/packages/core-api/src/lib/componentData.tsx new file mode 100644 index 0000000000..1f88356a48 --- /dev/null +++ b/packages/core-api/src/lib/componentData.tsx @@ -0,0 +1,69 @@ +/* + * Copyright 2020 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 { ComponentType, ReactNode } from 'react'; + +const DATA_KEY = Symbol('backstage-component-data'); + +type DataContainer = { + map: Map; +}; + +type ComponentWithData

= ComponentType

& { + [DATA_KEY]?: DataContainer; +}; + +type ReactNodeWithData = ReactNode & { + type?: { [DATA_KEY]?: DataContainer }; +}; + +export function attachComponentData

( + component: ComponentType

, + type: string, + data: unknown, +) { + const dataComponent = component as ComponentWithData

; + + let container = dataComponent[DATA_KEY]; + if (!container) { + container = dataComponent[DATA_KEY] = { map: new Map() }; + } + + if (container.map.has(type)) { + const name = component.displayName || component.name; + throw new Error( + `Attempted to attach duplicate data "${type}" to component "${name}"`, + ); + } + + container.map.set(type, data); +} + +export function getComponentData( + node: ReactNode, + type: string, +): T | undefined { + if (!node) { + return undefined; + } + + const container = (node as ReactNodeWithData).type?.[DATA_KEY]; + if (!container) { + return undefined; + } + + return container.map.get(type) as T | undefined; +} From 18821642ead8e5c0ca29692af6954426069a1770 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 25 Nov 2020 17:37:47 +0100 Subject: [PATCH 02/10] core-api: add plugin.provide and extensions + core extension functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam --- packages/core-api/src/lib/extensions.test.tsx | 72 +++++++++++++++++++ packages/core-api/src/lib/extensions.tsx | 59 +++++++++++++++ packages/core-api/src/plugin/Plugin.tsx | 13 +++- packages/core-api/src/plugin/types.ts | 5 ++ 4 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 packages/core-api/src/lib/extensions.test.tsx create mode 100644 packages/core-api/src/lib/extensions.tsx diff --git a/packages/core-api/src/lib/extensions.test.tsx b/packages/core-api/src/lib/extensions.test.tsx new file mode 100644 index 0000000000..92fcecebc4 --- /dev/null +++ b/packages/core-api/src/lib/extensions.test.tsx @@ -0,0 +1,72 @@ +/* + * Copyright 2020 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 { createPlugin } from '../plugin'; +import { createRouteRef } from '../routing'; +import { getComponentData } from './componentData'; +import { + createComponentExtension, + createReactExtension, + createRoutableExtension, +} from './extensions'; + +const plugin = createPlugin({ + id: 'my-plugin', +}); + +describe('extensions', () => { + it('should create a react extension with component data', () => { + const Component = () => null; + + const extension = createReactExtension({ + component: Component, + data: { + myData: { foo: 'bar' }, + }, + }); + + const ExtensionComponent = plugin.provide(extension); + const element = ; + + expect(getComponentData(element, 'core.plugin')).toBe(plugin); + expect(getComponentData(element, 'myData')).toEqual({ foo: 'bar' }); + }); + + it('should create react extensions of different types', () => { + const Component = () => null; + const routeRef = createRouteRef({ path: '/foo', title: 'Foo' }); + + const extension1 = createComponentExtension({ + component: Component, + }); + + const extension2 = createRoutableExtension({ + component: Component, + mountPoint: routeRef, + }); + + const ExtensionComponent1 = plugin.provide(extension1); + const ExtensionComponent2 = plugin.provide(extension2); + + const element1 = ; + const element2 = ; + + expect(getComponentData(element1, 'core.plugin')).toBe(plugin); + expect(getComponentData(element2, 'core.plugin')).toBe(plugin); + expect(getComponentData(element2, 'core.mountPoint')).toBe(routeRef); + }); +}); diff --git a/packages/core-api/src/lib/extensions.tsx b/packages/core-api/src/lib/extensions.tsx new file mode 100644 index 0000000000..c0a71a2a90 --- /dev/null +++ b/packages/core-api/src/lib/extensions.tsx @@ -0,0 +1,59 @@ +/* + * Copyright 2020 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, { ExoticComponent, ComponentType } from 'react'; +import { RouteRef } from '../routing'; +import { attachComponentData } from './componentData'; +import { Extension, BackstagePlugin } from '../plugin/types'; + +export function createRoutableExtension(options: { + component: ComponentType; + mountPoint: RouteRef; +}): Extension> { + const { component, mountPoint } = options; + return createReactExtension({ + component, + data: { + 'core.mountPoint': mountPoint, + }, + }); +} + +export function createComponentExtension(options: { + component: ComponentType; +}): Extension> { + const { component } = options; + return createReactExtension({ component }); +} + +export function createReactExtension(options: { + component: ComponentType; + data?: Record; +}): Extension> { + const { component: Component, data = {} } = options; + return { + expose(plugin: BackstagePlugin): ExoticComponent { + const Result = (props: Props) => ; + + attachComponentData(Result, 'core.plugin', plugin); + for (const [key, value] of Object.entries(data)) { + attachComponentData(Result, key, value); + } + + return Result as ExoticComponent; + }, + }; +} diff --git a/packages/core-api/src/plugin/Plugin.tsx b/packages/core-api/src/plugin/Plugin.tsx index d69bb7e721..f477674f4d 100644 --- a/packages/core-api/src/plugin/Plugin.tsx +++ b/packages/core-api/src/plugin/Plugin.tsx @@ -14,10 +14,15 @@ * limitations under the License. */ -import { PluginConfig, PluginOutput, BackstagePlugin } from './types'; +import { + PluginConfig, + PluginOutput, + BackstagePlugin, + Extension, +} from './types'; import { AnyApiFactory } from '../apis'; -export class PluginImpl { +export class PluginImpl implements BackstagePlugin { private storedOutput?: PluginOutput[]; constructor(private readonly config: PluginConfig) {} @@ -65,6 +70,10 @@ export class PluginImpl { return this.storedOutput; } + provide(extension: Extension): T { + return extension.expose(this); + } + toString() { return `plugin{${this.config.id}}`; } diff --git a/packages/core-api/src/plugin/types.ts b/packages/core-api/src/plugin/types.ts index 855264c5f4..dd19948a0a 100644 --- a/packages/core-api/src/plugin/types.ts +++ b/packages/core-api/src/plugin/types.ts @@ -66,10 +66,15 @@ export type PluginOutput = | RedirectRouteOutput | FeatureFlagOutput; +export type Extension = { + expose(plugin: BackstagePlugin): T; +}; + export type BackstagePlugin = { getId(): string; output(): PluginOutput[]; getApis(): Iterable; + provide(extension: Extension): T; }; export type PluginConfig = { From 632e1986166916e09d4bd8f2d88f2d7b6b14ac06 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 15:36:22 +0100 Subject: [PATCH 03/10] core-api: add mount point collection + extension tweaks Co-authored-by: blam --- packages/core-api/src/lib/extensions.tsx | 25 ++- .../core-api/src/routing/discovery.test.tsx | 145 ++++++++++++++++++ packages/core-api/src/routing/discovery.tsx | 75 +++++++++ 3 files changed, 239 insertions(+), 6 deletions(-) create mode 100644 packages/core-api/src/routing/discovery.test.tsx create mode 100644 packages/core-api/src/routing/discovery.tsx diff --git a/packages/core-api/src/lib/extensions.tsx b/packages/core-api/src/lib/extensions.tsx index c0a71a2a90..4087d5f963 100644 --- a/packages/core-api/src/lib/extensions.tsx +++ b/packages/core-api/src/lib/extensions.tsx @@ -14,7 +14,11 @@ * limitations under the License. */ -import React, { ExoticComponent, ComponentType } from 'react'; +import React, { + NamedExoticComponent, + ComponentType, + PropsWithChildren, +} from 'react'; import { RouteRef } from '../routing'; import { attachComponentData } from './componentData'; import { Extension, BackstagePlugin } from '../plugin/types'; @@ -22,7 +26,12 @@ import { Extension, BackstagePlugin } from '../plugin/types'; export function createRoutableExtension(options: { component: ComponentType; mountPoint: RouteRef; -}): Extension> { + // TODO(Rugvip): We want to carry forward the exact props type from the inner component, with + // or without children. ComponentType stops us from doing that though, as it always + // adds children to the props internally. We may want to work around this with custom types. +}): Extension< + NamedExoticComponent> +> { const { component, mountPoint } = options; return createReactExtension({ component, @@ -34,7 +43,7 @@ export function createRoutableExtension(options: { export function createComponentExtension(options: { component: ComponentType; -}): Extension> { +}): Extension> { const { component } = options; return createReactExtension({ component }); } @@ -42,10 +51,10 @@ export function createComponentExtension(options: { export function createReactExtension(options: { component: ComponentType; data?: Record; -}): Extension> { +}): Extension> { const { component: Component, data = {} } = options; return { - expose(plugin: BackstagePlugin): ExoticComponent { + expose(plugin: BackstagePlugin): NamedExoticComponent { const Result = (props: Props) => ; attachComponentData(Result, 'core.plugin', plugin); @@ -53,7 +62,11 @@ export function createReactExtension(options: { attachComponentData(Result, key, value); } - return Result as ExoticComponent; + const name = Component.displayName || Component.name || 'Component'; + if (name) { + Result.displayName = `Extension(${name})`; + } + return Result as NamedExoticComponent; }, }; } diff --git a/packages/core-api/src/routing/discovery.test.tsx b/packages/core-api/src/routing/discovery.test.tsx new file mode 100644 index 0000000000..c894c8ac6d --- /dev/null +++ b/packages/core-api/src/routing/discovery.test.tsx @@ -0,0 +1,145 @@ +/* + * Copyright 2020 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, { PropsWithChildren } from 'react'; +import { collectRoutes } from './discovery'; +import { createRouteRef } from './RouteRef'; +import { createPlugin } from '../plugin'; +import { createRoutableExtension } from '../lib/extensions'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; + +const mockConfig = () => ({ path: '/foo', title: 'Foo' }); +const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; + +const plugin = createPlugin({ id: 'my-plugin' }); + +const ref1 = createRouteRef(mockConfig()); +const ref2 = createRouteRef(mockConfig()); +const ref3 = createRouteRef(mockConfig()); +const ref4 = createRouteRef(mockConfig()); +const ref5 = createRouteRef(mockConfig()); + +const Extension1 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref1 }), +); +const Extension2 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref2 }), +); +const Extension3 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref3 }), +); +const Extension4 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref4 }), +); +const Extension5 = plugin.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref5 }), +); + +describe('discovery', () => { + it('should collect routes', () => { + const list = [ +

, +
, +
+ +
, + ]; + + const routes = collectRoutes( + + + +
+ +
+
+ Some text here shouldn't be a problem +
+ {null} +
+ +
+ + {false} + {list} + {true} + {0} +
+ +
+ } /> +
+ + , + ); + + expect(routes).toEqual( + new Map([ + [ref1, '/foo'], + [ref2, '/bar/:id'], + [ref3, '/baz'], + [ref4, '/divsoup'], + [ref5, '/blop'], + ]), + ); + }); + + it('should handle all react router Route patterns', () => { + const routes = collectRoutes( + + + + + + + + } + /> + }> + } /> + + + + , + ); + + expect(routes).toEqual( + new Map([ + [ref1, '/foo'], + [ref2, '/bar/:id'], + [ref3, '/baz'], + [ref4, '/divsoup'], + [ref5, '/blop'], + ]), + ); + }); + + it('should not visit the same element twice', () => { + const element = ; + + expect(() => + collectRoutes( + + {element} + {element} + , + ), + ).toThrow(`Visited element Extension(MockComponent) twice`); + }); +}); diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/routing/discovery.tsx new file mode 100644 index 0000000000..61e5df9136 --- /dev/null +++ b/packages/core-api/src/routing/discovery.tsx @@ -0,0 +1,75 @@ +/* + * Copyright 2020 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, { isValidElement, ReactNode } from 'react'; +import { RouteRef } from './types'; +import { getComponentData } from '../lib/componentData'; + +export const collectRoutes = (tree: ReactNode) => { + const treeMap = new Map(); + + const visited = new Set(); + const nodes = [tree]; + + while (nodes.length !== 0) { + const node = nodes.shift(); + if (!isIterableElement(node)) { + continue; + } + if (visited.has(node)) { + const anyType = node?.type as + | { displayName?: string; name?: string } + | undefined; + const name = anyType?.displayName || anyType?.name || String(anyType); + throw new Error(`Visited element ${name} twice`); + } + visited.add(node); + + React.Children.forEach(node, child => { + if (!isIterableElement(child)) { + return; + } + + const { path, element, children } = child.props as { + path?: string; + element?: ReactNode; + children?: ReactNode; + }; + if (path) { + const routeRef = getComponentData(child, 'core.mountPoint'); + if (routeRef) { + treeMap.set(routeRef, path); + } else if (isIterableElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); + if (elementRouteRef) { + treeMap.set(elementRouteRef, path); + } + nodes.push(element.props?.children); + } + } + nodes.push(children); + }); + } + + return treeMap; +}; + +function isIterableElement(node: ReactNode): node is JSX.Element { + return isValidElement(node) || Array.isArray(node); +} From 2009082476983189b9cfe29a7b93b1de21b9d3b9 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 16:07:03 +0100 Subject: [PATCH 04/10] core-api: add mount point parent discovery Co-authored-by: blam --- .../core-api/src/routing/discovery.test.tsx | 33 +++++++++--- packages/core-api/src/routing/discovery.tsx | 54 +++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) diff --git a/packages/core-api/src/routing/discovery.test.tsx b/packages/core-api/src/routing/discovery.test.tsx index c894c8ac6d..02c80297c0 100644 --- a/packages/core-api/src/routing/discovery.test.tsx +++ b/packages/core-api/src/routing/discovery.test.tsx @@ -15,7 +15,7 @@ */ import React, { PropsWithChildren } from 'react'; -import { collectRoutes } from './discovery'; +import { collectRoutes, collectRouteParents } from './discovery'; import { createRouteRef } from './RouteRef'; import { createPlugin } from '../plugin'; import { createRoutableExtension } from '../lib/extensions'; @@ -58,7 +58,7 @@ describe('discovery', () => {
, ]; - const routes = collectRoutes( + const root = ( @@ -83,10 +83,10 @@ describe('discovery', () => { } />
- , + ); - expect(routes).toEqual( + expect(collectRoutes(root)).toEqual( new Map([ [ref1, '/foo'], [ref2, '/bar/:id'], @@ -95,10 +95,20 @@ describe('discovery', () => { [ref5, '/blop'], ]), ); + + expect(collectRouteParents(root)).toEqual( + new Map([ + [ref1, undefined], + [ref2, ref1], + [ref3, ref2], + [ref4, undefined], + [ref5, ref1], + ]), + ); }); it('should handle all react router Route patterns', () => { - const routes = collectRoutes( + const root = ( { - , + ); - expect(routes).toEqual( + expect(collectRoutes(root)).toEqual( new Map([ [ref1, '/foo'], [ref2, '/bar/:id'], @@ -128,6 +138,15 @@ describe('discovery', () => { [ref5, '/blop'], ]), ); + expect(collectRouteParents(root)).toEqual( + new Map([ + [ref1, undefined], + [ref2, ref1], + [ref3, undefined], + [ref4, ref3], + [ref5, ref3], + ]), + ); }); it('should not visit the same element twice', () => { diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/routing/discovery.tsx index 61e5df9136..96946e1ca3 100644 --- a/packages/core-api/src/routing/discovery.tsx +++ b/packages/core-api/src/routing/discovery.tsx @@ -70,6 +70,60 @@ export const collectRoutes = (tree: ReactNode) => { return treeMap; }; +export const collectRouteParents = (tree: ReactNode) => { + const treeMap = new Map(); + + const nodes = [{ node: tree, parent: undefined as RouteRef | undefined }]; + + while (nodes.length !== 0) { + const { parent, node } = nodes.shift()!; + if (!isIterableElement(node)) { + continue; + } + + React.Children.forEach(node, child => { + if (!isIterableElement(child)) { + return; + } + + const { path, element, children } = child.props as { + path?: string; + element?: ReactNode; + children?: ReactNode; + }; + + let nextParent = parent; + + if (path) { + const routeRef = getComponentData(child, 'core.mountPoint'); + if (routeRef) { + treeMap.set(routeRef, parent); + nextParent = routeRef; + } else if (isIterableElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); + if (elementRouteRef) { + treeMap.set(elementRouteRef, parent); + + nextParent = elementRouteRef; + nodes.push({ + parent: elementRouteRef, + node: element.props?.children, + }); + } else { + nodes.push({ parent, node: element.props?.children }); + } + } + } + nodes.push({ parent: nextParent, node: children }); + }); + } + + return treeMap; +}; + function isIterableElement(node: ReactNode): node is JSX.Element { return isValidElement(node) || Array.isArray(node); } From e7bf831061532baa8d06f9776c1eaa4a67427e7c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 16:48:50 +0100 Subject: [PATCH 05/10] core-api: add plugin instance collection utility Co-authored-by: blam --- .../core-api/src/plugin/collection.test.tsx | 85 +++++++++++++++++++ packages/core-api/src/plugin/collection.ts | 72 ++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 packages/core-api/src/plugin/collection.test.tsx create mode 100644 packages/core-api/src/plugin/collection.ts diff --git a/packages/core-api/src/plugin/collection.test.tsx b/packages/core-api/src/plugin/collection.test.tsx new file mode 100644 index 0000000000..cb123cd2b1 --- /dev/null +++ b/packages/core-api/src/plugin/collection.test.tsx @@ -0,0 +1,85 @@ +/* + * Copyright 2020 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, { PropsWithChildren } from 'react'; +import { createRouteRef } from '../routing'; +import { createPlugin } from '../plugin'; +import { + createRoutableExtension, + createComponentExtension, +} from '../lib/extensions'; +import { MemoryRouter, Routes, Route } from 'react-router-dom'; +import { collectPlugins } from './collection'; + +const mockConfig = () => ({ path: '/foo', title: 'Foo' }); +const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; + +const pluginA = createPlugin({ id: 'my-plugin-a' }); +const pluginB = createPlugin({ id: 'my-plugin-b' }); +const pluginC = createPlugin({ id: 'my-plugin-c' }); + +const ref1 = createRouteRef(mockConfig()); +const ref2 = createRouteRef(mockConfig()); + +const Extension1 = pluginA.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref1 }), +); +const Extension2 = pluginB.provide( + createRoutableExtension({ component: MockComponent, mountPoint: ref2 }), +); +const Extension3 = pluginA.provide( + createComponentExtension({ component: MockComponent }), +); +const Extension4 = pluginB.provide( + createComponentExtension({ component: MockComponent }), +); +const Extension5 = pluginC.provide( + createComponentExtension({ component: MockComponent }), +); + +describe('collection', () => { + it('should collect the plugins', () => { + const root = ( + + + +
+ +
+
+ {[]} + Some text here shouldn't be a problem +
+ {null} +
+ +
+ + {false} + {true} + {0} +
+ +
+ } /> +
+ + + ); + + expect(collectPlugins(root)).toEqual(new Set([pluginA, pluginB, pluginC])); + }); +}); diff --git a/packages/core-api/src/plugin/collection.ts b/packages/core-api/src/plugin/collection.ts new file mode 100644 index 0000000000..23139deac4 --- /dev/null +++ b/packages/core-api/src/plugin/collection.ts @@ -0,0 +1,72 @@ +/* + * Copyright 2020 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. + */ +/* + * Copyright 2020 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, { isValidElement, ReactNode } from 'react'; +import { BackstagePlugin } from './types'; +import { getComponentData } from '../lib/componentData'; + +export const collectPlugins = (tree: ReactNode) => { + const plugins = new Set(); + + const nodes = [tree]; + + while (nodes.length !== 0) { + const node = nodes.shift(); + if (!isIterableElement(node)) { + continue; + } + + React.Children.forEach(node, child => { + if (!isIterableElement(child)) { + return; + } + + const { element, children } = child.props as { + element?: ReactNode; + children?: ReactNode; + }; + const plugin = getComponentData(child, 'core.plugin'); + if (plugin) { + plugins.add(plugin); + } + if (isIterableElement(element)) { + nodes.push(element); + } + nodes.push(children); + }); + } + + return plugins; +}; + +function isIterableElement(node: ReactNode): node is JSX.Element { + return isValidElement(node) || Array.isArray(node); +} From 9cf15f6baa62bf08a506f2eaf91e8995b1f910d6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 16:54:36 +0100 Subject: [PATCH 06/10] core-api: move extension and component data utils into extensions/ Co-authored-by: blam --- .../componentData.test.tsx | 0 .../src/{lib => extensions}/componentData.tsx | 0 .../{lib => extensions}/extensions.test.tsx | 0 .../src/{lib => extensions}/extensions.tsx | 0 packages/core-api/src/extensions/index.ts | 22 +++++++++++++++++++ .../core-api/src/plugin/collection.test.tsx | 2 +- packages/core-api/src/plugin/collection.ts | 2 +- .../core-api/src/routing/discovery.test.tsx | 2 +- packages/core-api/src/routing/discovery.tsx | 2 +- 9 files changed, 26 insertions(+), 4 deletions(-) rename packages/core-api/src/{lib => extensions}/componentData.test.tsx (100%) rename packages/core-api/src/{lib => extensions}/componentData.tsx (100%) rename packages/core-api/src/{lib => extensions}/extensions.test.tsx (100%) rename packages/core-api/src/{lib => extensions}/extensions.tsx (100%) create mode 100644 packages/core-api/src/extensions/index.ts diff --git a/packages/core-api/src/lib/componentData.test.tsx b/packages/core-api/src/extensions/componentData.test.tsx similarity index 100% rename from packages/core-api/src/lib/componentData.test.tsx rename to packages/core-api/src/extensions/componentData.test.tsx diff --git a/packages/core-api/src/lib/componentData.tsx b/packages/core-api/src/extensions/componentData.tsx similarity index 100% rename from packages/core-api/src/lib/componentData.tsx rename to packages/core-api/src/extensions/componentData.tsx diff --git a/packages/core-api/src/lib/extensions.test.tsx b/packages/core-api/src/extensions/extensions.test.tsx similarity index 100% rename from packages/core-api/src/lib/extensions.test.tsx rename to packages/core-api/src/extensions/extensions.test.tsx diff --git a/packages/core-api/src/lib/extensions.tsx b/packages/core-api/src/extensions/extensions.tsx similarity index 100% rename from packages/core-api/src/lib/extensions.tsx rename to packages/core-api/src/extensions/extensions.tsx diff --git a/packages/core-api/src/extensions/index.ts b/packages/core-api/src/extensions/index.ts new file mode 100644 index 0000000000..26a0c597b1 --- /dev/null +++ b/packages/core-api/src/extensions/index.ts @@ -0,0 +1,22 @@ +/* + * Copyright 2020 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. + */ + +export { attachComponentData, getComponentData } from './componentData'; +export { + createReactExtension, + createRoutableExtension, + createComponentExtension, +} from './extensions'; diff --git a/packages/core-api/src/plugin/collection.test.tsx b/packages/core-api/src/plugin/collection.test.tsx index cb123cd2b1..fd94303ef4 100644 --- a/packages/core-api/src/plugin/collection.test.tsx +++ b/packages/core-api/src/plugin/collection.test.tsx @@ -20,7 +20,7 @@ import { createPlugin } from '../plugin'; import { createRoutableExtension, createComponentExtension, -} from '../lib/extensions'; +} from '../extensions'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; import { collectPlugins } from './collection'; diff --git a/packages/core-api/src/plugin/collection.ts b/packages/core-api/src/plugin/collection.ts index 23139deac4..0811f66445 100644 --- a/packages/core-api/src/plugin/collection.ts +++ b/packages/core-api/src/plugin/collection.ts @@ -31,7 +31,7 @@ import React, { isValidElement, ReactNode } from 'react'; import { BackstagePlugin } from './types'; -import { getComponentData } from '../lib/componentData'; +import { getComponentData } from '../extensions'; export const collectPlugins = (tree: ReactNode) => { const plugins = new Set(); diff --git a/packages/core-api/src/routing/discovery.test.tsx b/packages/core-api/src/routing/discovery.test.tsx index 02c80297c0..be553d9ce5 100644 --- a/packages/core-api/src/routing/discovery.test.tsx +++ b/packages/core-api/src/routing/discovery.test.tsx @@ -18,7 +18,7 @@ import React, { PropsWithChildren } from 'react'; import { collectRoutes, collectRouteParents } from './discovery'; import { createRouteRef } from './RouteRef'; import { createPlugin } from '../plugin'; -import { createRoutableExtension } from '../lib/extensions'; +import { createRoutableExtension } from '../extensions'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; const mockConfig = () => ({ path: '/foo', title: 'Foo' }); diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/routing/discovery.tsx index 96946e1ca3..973c96e420 100644 --- a/packages/core-api/src/routing/discovery.tsx +++ b/packages/core-api/src/routing/discovery.tsx @@ -16,7 +16,7 @@ import React, { isValidElement, ReactNode } from 'react'; import { RouteRef } from './types'; -import { getComponentData } from '../lib/componentData'; +import { getComponentData } from '../extensions'; export const collectRoutes = (tree: ReactNode) => { const treeMap = new Map(); From 3bc19b75e8783f0c07bf58a20ae7a2ea795834b3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 17:48:32 +0100 Subject: [PATCH 07/10] core-api: refactor route discovery to use common traversal function Co-authored-by: blam --- packages/core-api/src/routing/discovery.tsx | 166 ++++++++++---------- 1 file changed, 87 insertions(+), 79 deletions(-) diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/routing/discovery.tsx index 973c96e420..65983a8a77 100644 --- a/packages/core-api/src/routing/discovery.tsx +++ b/packages/core-api/src/routing/discovery.tsx @@ -18,54 +18,75 @@ import React, { isValidElement, ReactNode } from 'react'; import { RouteRef } from './types'; import { getComponentData } from '../extensions'; -export const collectRoutes = (tree: ReactNode) => { - const treeMap = new Map(); +type TraverseFunc = ( + node: JSX.Element, + context: Context, +) => Generator< + Context extends undefined + ? { children: JSX.Element; context?: Context } + : { children: JSX.Element; context: Context }, + void, + void +>; +function traverse( + root: ReactNode, + initialContext: Context, + traverseFunc: TraverseFunc, +): void { const visited = new Set(); - const nodes = [tree]; + const nodes = [{ children: root, context: initialContext }]; while (nodes.length !== 0) { - const node = nodes.shift(); - if (!isIterableElement(node)) { - continue; - } - if (visited.has(node)) { - const anyType = node?.type as - | { displayName?: string; name?: string } - | undefined; - const name = anyType?.displayName || anyType?.name || String(anyType); - throw new Error(`Visited element ${name} twice`); - } - visited.add(node); + const { children, context } = nodes.shift()!; - React.Children.forEach(node, child => { + React.Children.forEach(children, child => { if (!isIterableElement(child)) { return; } - - const { path, element, children } = child.props as { - path?: string; - element?: ReactNode; - children?: ReactNode; - }; - if (path) { - const routeRef = getComponentData(child, 'core.mountPoint'); - if (routeRef) { - treeMap.set(routeRef, path); - } else if (isIterableElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - if (elementRouteRef) { - treeMap.set(elementRouteRef, path); - } - nodes.push(element.props?.children); - } + if (visited.has(child)) { + const anyType = child?.type as + | { displayName?: string; name?: string } + | undefined; + const name = anyType?.displayName || anyType?.name || String(anyType); + throw new Error(`Visited element ${name} twice`); + } + visited.add(child); + + for (const next of traverseFunc(child, context)) { + nodes.push(next as { children: JSX.Element; context: Context }); } - nodes.push(children); }); } +} + +export const collectRoutes = (tree: ReactNode) => { + const treeMap = new Map(); + + traverse(tree, undefined, function* traverseFunc(node) { + const { path, element, children } = node.props as { + path?: string; + element?: ReactNode; + children?: ReactNode; + }; + if (path) { + const routeRef = getComponentData(node, 'core.mountPoint'); + if (routeRef) { + treeMap.set(routeRef, path); + } else if (isIterableElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); + if (elementRouteRef) { + treeMap.set(elementRouteRef, path); + } + yield { children: element.props?.children }; + } + } + + yield { children }; + }); return treeMap; }; @@ -73,53 +94,40 @@ export const collectRoutes = (tree: ReactNode) => { export const collectRouteParents = (tree: ReactNode) => { const treeMap = new Map(); - const nodes = [{ node: tree, parent: undefined as RouteRef | undefined }]; + traverse(tree, undefined, function* traverseFunc( + node, + parent, + ) { + const { path, element, children } = node.props as { + path?: string; + element?: ReactNode; + children?: ReactNode; + }; - while (nodes.length !== 0) { - const { parent, node } = nodes.shift()!; - if (!isIterableElement(node)) { - continue; - } + let nextParent = parent; - React.Children.forEach(node, child => { - if (!isIterableElement(child)) { - return; - } + if (path) { + const routeRef = getComponentData(node, 'core.mountPoint'); + if (routeRef) { + treeMap.set(routeRef, parent); + nextParent = routeRef; + } else if (isIterableElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); - const { path, element, children } = child.props as { - path?: string; - element?: ReactNode; - children?: ReactNode; - }; - - let nextParent = parent; - - if (path) { - const routeRef = getComponentData(child, 'core.mountPoint'); - if (routeRef) { - treeMap.set(routeRef, parent); - nextParent = routeRef; - } else if (isIterableElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - if (elementRouteRef) { - treeMap.set(elementRouteRef, parent); - - nextParent = elementRouteRef; - nodes.push({ - parent: elementRouteRef, - node: element.props?.children, - }); - } else { - nodes.push({ parent, node: element.props?.children }); - } + if (elementRouteRef) { + treeMap.set(elementRouteRef, parent); + nextParent = elementRouteRef; + yield { children: element.props?.children, context: elementRouteRef }; + } else { + yield { children: element.props?.children, context: parent }; } } - nodes.push({ parent: nextParent, node: children }); - }); - } + } + yield { children, context: nextParent }; + }); return treeMap; }; From 9cced997b22930df77521d586979d227a462b856 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 18:12:02 +0100 Subject: [PATCH 08/10] core-api: refactor routing element traversal to not skip over elements Co-authored-by: blam --- packages/core-api/src/routing/discovery.tsx | 123 +++++++++++--------- 1 file changed, 71 insertions(+), 52 deletions(-) diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/routing/discovery.tsx index 65983a8a77..b925c8349d 100644 --- a/packages/core-api/src/routing/discovery.tsx +++ b/packages/core-api/src/routing/discovery.tsx @@ -14,34 +14,44 @@ * limitations under the License. */ -import React, { isValidElement, ReactNode } from 'react'; +import { isValidElement, ReactNode, ReactElement, Children } from 'react'; import { RouteRef } from './types'; import { getComponentData } from '../extensions'; type TraverseFunc = ( - node: JSX.Element, + node: ReactElement, + parent: ReactElement, context: Context, ) => Generator< Context extends undefined - ? { children: JSX.Element; context?: Context } - : { children: JSX.Element; context: Context }, + ? { children: ReactNode; context?: Context } + : { children: ReactNode; context: Context }, void, void >; -function traverse( - root: ReactNode, - initialContext: Context, +function traverse( + options: { root: ReactElement; initialContext?: Context }, traverseFunc: TraverseFunc, ): void { const visited = new Set(); - const nodes = [{ children: root, context: initialContext }]; + const nodes: Array<{ + children: ReactNode; + parent: ReactElement; + context: Context; + }> = [ + { + children: Children.toArray(options.root), + parent: options.root, + context: options.initialContext!, + }, + ]; while (nodes.length !== 0) { - const { children, context } = nodes.shift()!; + const { children, parent, context } = nodes.shift()!; - React.Children.forEach(children, child => { - if (!isIterableElement(child)) { + Children.forEach(children, child => { + if (!isValidElement(child)) { return; } if (visited.has(child)) { @@ -53,36 +63,48 @@ function traverse( } visited.add(child); - for (const next of traverseFunc(child, context)) { - nodes.push(next as { children: JSX.Element; context: Context }); + for (const next of traverseFunc(child, parent, context)) { + nodes.push({ + children: next.children, + context: next.context!, + parent: child, + }); } }); } } -export const collectRoutes = (tree: ReactNode) => { +export const collectRoutes = (root: ReactElement) => { const treeMap = new Map(); - traverse(tree, undefined, function* traverseFunc(node) { + traverse({ root }, function* traverseFunc(node, parent: ReactElement) { const { path, element, children } = node.props as { path?: string; element?: ReactNode; children?: ReactNode; }; - if (path) { - const routeRef = getComponentData(node, 'core.mountPoint'); - if (routeRef) { - treeMap.set(routeRef, path); - } else if (isIterableElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - if (elementRouteRef) { - treeMap.set(elementRouteRef, path); - } - yield { children: element.props?.children }; + if (parent.props.element === node) { + yield { children }; + return; + } + const routeRef = getComponentData(node, 'core.mountPoint'); + if (routeRef) { + if (!path) { + throw new Error('Mounted routable extension must have a path'); } + treeMap.set(routeRef, path); + } else if (isValidElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); + if (elementRouteRef) { + if (!path) { + throw new Error('Route element must have a path'); + } + treeMap.set(elementRouteRef, path); + } + yield { children: element }; } yield { children }; @@ -91,47 +113,44 @@ export const collectRoutes = (tree: ReactNode) => { return treeMap; }; -export const collectRouteParents = (tree: ReactNode) => { +export const collectRouteParents = (root: ReactElement) => { const treeMap = new Map(); - traverse(tree, undefined, function* traverseFunc( - node, - parent, - ) { - const { path, element, children } = node.props as { - path?: string; - element?: ReactNode; - children?: ReactNode; - }; + traverse( + { root, initialContext: undefined }, + function* traverseFunc(node, parent, parentRouteRef) { + const { element, children } = node.props as { + element?: ReactNode; + children?: ReactNode; + }; + if (parent.props.element === node) { + yield { children }; + return; + } - let nextParent = parent; + let nextParent = parentRouteRef; - if (path) { const routeRef = getComponentData(node, 'core.mountPoint'); if (routeRef) { - treeMap.set(routeRef, parent); + treeMap.set(routeRef, parentRouteRef); nextParent = routeRef; - } else if (isIterableElement(element)) { + } else if (isValidElement(element)) { const elementRouteRef = getComponentData( element, 'core.mountPoint', ); if (elementRouteRef) { - treeMap.set(elementRouteRef, parent); + treeMap.set(elementRouteRef, parentRouteRef); nextParent = elementRouteRef; - yield { children: element.props?.children, context: elementRouteRef }; + yield { children: element, context: elementRouteRef }; } else { - yield { children: element.props?.children, context: parent }; + yield { children: element, context: parentRouteRef }; } } - } - yield { children, context: nextParent }; - }); + yield { children, context: nextParent }; + }, + ); return treeMap; }; - -function isIterableElement(node: ReactNode): node is JSX.Element { - return isValidElement(node) || Array.isArray(node); -} From 706868141eafb9d5e5626ad29595b6d5e5ba979e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 26 Nov 2020 19:22:59 +0100 Subject: [PATCH 09/10] core-api: refactor route discovery to do a single pluggable sweep --- .../core-api/src/routing/discovery.test.tsx | 51 +++- packages/core-api/src/routing/discovery.tsx | 224 +++++++++++------- 2 files changed, 175 insertions(+), 100 deletions(-) diff --git a/packages/core-api/src/routing/discovery.test.tsx b/packages/core-api/src/routing/discovery.test.tsx index be553d9ce5..c702811529 100644 --- a/packages/core-api/src/routing/discovery.test.tsx +++ b/packages/core-api/src/routing/discovery.test.tsx @@ -15,7 +15,13 @@ */ import React, { PropsWithChildren } from 'react'; -import { collectRoutes, collectRouteParents } from './discovery'; +import { + traverseElementTree, + childDiscoverer, + routeElementDiscoverer, + routeCollector, + routeParentCollector, +} from './discovery'; import { createRouteRef } from './RouteRef'; import { createPlugin } from '../plugin'; import { createRoutableExtension } from '../extensions'; @@ -86,7 +92,15 @@ describe('discovery', () => { ); - expect(collectRoutes(root)).toEqual( + const { routes, routeParents } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }); + expect(routes).toEqual( new Map([ [ref1, '/foo'], [ref2, '/bar/:id'], @@ -96,7 +110,7 @@ describe('discovery', () => { ]), ); - expect(collectRouteParents(root)).toEqual( + expect(routeParents).toEqual( new Map([ [ref1, undefined], [ref2, ref1], @@ -129,7 +143,15 @@ describe('discovery', () => { ); - expect(collectRoutes(root)).toEqual( + const { routes, routeParents } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }); + expect(routes).toEqual( new Map([ [ref1, '/foo'], [ref2, '/bar/:id'], @@ -138,7 +160,7 @@ describe('discovery', () => { [ref5, '/blop'], ]), ); - expect(collectRouteParents(root)).toEqual( + expect(routeParents).toEqual( new Map([ [ref1, undefined], [ref2, ref1], @@ -153,12 +175,19 @@ describe('discovery', () => { const element = ; expect(() => - collectRoutes( - - {element} - {element} - , - ), + traverseElementTree({ + root: ( + + {element} + {element} + + ), + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + routes: routeCollector, + routeParents: routeParentCollector, + }, + }), ).toThrow(`Visited element Extension(MockComponent) twice`); }); }); diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/routing/discovery.tsx index b925c8349d..0189ae3a4e 100644 --- a/packages/core-api/src/routing/discovery.tsx +++ b/packages/core-api/src/routing/discovery.tsx @@ -18,81 +18,143 @@ import { isValidElement, ReactNode, ReactElement, Children } from 'react'; import { RouteRef } from './types'; import { getComponentData } from '../extensions'; -type TraverseFunc = ( - node: ReactElement, - parent: ReactElement, - context: Context, -) => Generator< - Context extends undefined - ? { children: ReactNode; context?: Context } - : { children: ReactNode; context: Context }, - void, - void ->; +type Discoverer = (element: ReactElement) => ReactNode; -function traverse( - options: { root: ReactElement; initialContext?: Context }, - traverseFunc: TraverseFunc, -): void { +type Collector = () => { + accumulator: Result; + visit( + accumulator: Result, + element: ReactElement, + parent: ReactElement, + context: Context, + ): Context; +}; + +/** + * A function that allows you to traverse a tree of React elements using + * varying methods to discover child nodes and collect data along the way. + */ +export function traverseElementTree(options: { + root: ReactElement; + discoverers: Discoverer[]; + collectors: { [name in keyof Results]: Collector }; +}): Results { const visited = new Set(); - const nodes: Array<{ - children: ReactNode; + const collectors: { + [name in string]: ReturnType>; + } = {}; + + // Bootstrap all collectors, initializing the accumulators and providing the visitor function + for (const name in options.collectors) { + if (options.collectors.hasOwnProperty(name)) { + collectors[name] = options.collectors[name](); + } + } + + // Internal representation of an element in the tree that we're iterating over + type QueueItem = { + node: ReactNode; parent: ReactElement; - context: Context; - }> = [ + contexts: { [name in string]: unknown }; + }; + + const queue = [ { - children: Children.toArray(options.root), + node: Children.toArray(options.root), parent: options.root, - context: options.initialContext!, - }, + contexts: {}, + } as QueueItem, ]; - while (nodes.length !== 0) { - const { children, parent, context } = nodes.shift()!; + while (queue.length !== 0) { + const { node, parent, contexts } = queue.shift()!; - Children.forEach(children, child => { - if (!isValidElement(child)) { + // While the parent and the element we pass on to collectors and discoverers + // have been validated and are known to be React elements, the child nodes + // emitted by the discoverers are not. + Children.forEach(node, element => { + if (!isValidElement(element)) { return; } - if (visited.has(child)) { - const anyType = child?.type as + if (visited.has(element)) { + const anyType = element?.type as | { displayName?: string; name?: string } | undefined; const name = anyType?.displayName || anyType?.name || String(anyType); throw new Error(`Visited element ${name} twice`); } - visited.add(child); + visited.add(element); - for (const next of traverseFunc(child, parent, context)) { - nodes.push({ - children: next.children, - context: next.context!, - parent: child, - }); + const nextContexts: QueueItem['contexts'] = {}; + + // Collectors populate their result data using the current node, and compute + // context for the next iteration + for (const name in collectors) { + if (collectors.hasOwnProperty(name)) { + const collector = collectors[name]; + + nextContexts[name] = collector.visit( + collector.accumulator, + element, + parent, + contexts[name], + ); + } + } + + // Discoverers provide ways to continue the traversal from the current element + for (const discoverer of options.discoverers) { + const children = discoverer(element); + if (children) { + queue.push({ + node: children, + parent: element, + contexts: nextContexts, + }); + } } }); } + + return Object.fromEntries( + Object.entries(collectors).map(([name, c]) => [name, c.accumulator]), + ) as Results; } -export const collectRoutes = (root: ReactElement) => { - const treeMap = new Map(); +export function childDiscoverer(element: ReactElement): ReactNode { + return element.props?.children; +} - traverse({ root }, function* traverseFunc(node, parent: ReactElement) { - const { path, element, children } = node.props as { - path?: string; - element?: ReactNode; - children?: ReactNode; - }; +export function routeElementDiscoverer(element: ReactElement): ReactNode { + if (element.props?.path && element.props?.element) { + return element.props?.element; + } + return undefined; +} + +function createCollector( + initialResult: Result, + visit: ReturnType>['visit'], +): Collector { + return () => ({ accumulator: initialResult, visit }); +} + +export const routeCollector = createCollector( + new Map(), + (acc, node, parent) => { if (parent.props.element === node) { - yield { children }; return; } + + const path: string | undefined = node.props?.path; + const element: ReactNode = node.props?.element; + const routeRef = getComponentData(node, 'core.mountPoint'); if (routeRef) { if (!path) { throw new Error('Mounted routable extension must have a path'); } - treeMap.set(routeRef, path); + acc.set(routeRef, path); } else if (isValidElement(element)) { const elementRouteRef = getComponentData( element, @@ -102,55 +164,39 @@ export const collectRoutes = (root: ReactElement) => { if (!path) { throw new Error('Route element must have a path'); } - treeMap.set(elementRouteRef, path); + acc.set(elementRouteRef, path); } - yield { children: element }; + } + }, +); + +export const routeParentCollector = createCollector( + new Map(), + (acc, node, parent, parentRouteRef?: RouteRef) => { + if (parent.props.element === node) { + return parentRouteRef; } - yield { children }; - }); + const element: ReactNode = node.props?.element; - return treeMap; -}; + let nextParent = parentRouteRef; -export const collectRouteParents = (root: ReactElement) => { - const treeMap = new Map(); + const routeRef = getComponentData(node, 'core.mountPoint'); + if (routeRef) { + acc.set(routeRef, parentRouteRef); + nextParent = routeRef; + } else if (isValidElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); - traverse( - { root, initialContext: undefined }, - function* traverseFunc(node, parent, parentRouteRef) { - const { element, children } = node.props as { - element?: ReactNode; - children?: ReactNode; - }; - if (parent.props.element === node) { - yield { children }; - return; + if (elementRouteRef) { + acc.set(elementRouteRef, parentRouteRef); + nextParent = elementRouteRef; } + } - let nextParent = parentRouteRef; - - const routeRef = getComponentData(node, 'core.mountPoint'); - if (routeRef) { - treeMap.set(routeRef, parentRouteRef); - nextParent = routeRef; - } else if (isValidElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - - if (elementRouteRef) { - treeMap.set(elementRouteRef, parentRouteRef); - nextParent = elementRouteRef; - yield { children: element, context: elementRouteRef }; - } else { - yield { children: element, context: parentRouteRef }; - } - } - yield { children, context: nextParent }; - }, - ); - - return treeMap; -}; + return nextParent; + }, +); From 26ed08c1679de2ccc8205f417a83889a90002133 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 27 Nov 2020 11:41:59 +0100 Subject: [PATCH 10/10] core-api: refactor all element data collection to use the same traversal facilities Co-authored-by: blam --- .../src/extensions/traversal.test.tsx | 98 +++++++++++++++++++ .../discovery.tsx => extensions/traversal.ts} | 82 ++-------------- ...ollection.test.tsx => collectors.test.tsx} | 19 +++- .../plugin/{collection.ts => collectors.ts} | 46 ++------- ...discovery.test.tsx => collectors.test.tsx} | 6 +- packages/core-api/src/routing/collectors.tsx | 82 ++++++++++++++++ 6 files changed, 217 insertions(+), 116 deletions(-) create mode 100644 packages/core-api/src/extensions/traversal.test.tsx rename packages/core-api/src/{routing/discovery.tsx => extensions/traversal.ts} (68%) rename packages/core-api/src/plugin/{collection.test.tsx => collectors.test.tsx} (84%) rename packages/core-api/src/plugin/{collection.ts => collectors.ts} (58%) rename packages/core-api/src/routing/{discovery.test.tsx => collectors.test.tsx} (98%) create mode 100644 packages/core-api/src/routing/collectors.tsx diff --git a/packages/core-api/src/extensions/traversal.test.tsx b/packages/core-api/src/extensions/traversal.test.tsx new file mode 100644 index 0000000000..a69ee9de77 --- /dev/null +++ b/packages/core-api/src/extensions/traversal.test.tsx @@ -0,0 +1,98 @@ +/* + * Copyright 2020 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, { Children, isValidElement } from 'react'; +import { + childDiscoverer, + createCollector, + traverseElementTree, +} from './traversal'; + +describe('discovery', () => { + it('should collect element names', () => { + const root = ( +
+
+

Title

+

Text

+
+
+
+

Title

+ Text +
+
+ ); + + const { names } = traverseElementTree({ + root, + discoverers: [childDiscoverer], + collectors: { + names: createCollector(Array(), (acc, el) => { + if (typeof el.type === 'string') { + acc.push(el.type); + } + }), + }, + }); + + expect(names).toEqual([ + 'main', + 'div', + 'hr', + 'div', + 'h1', + 'p', + 'h2', + 'span', + ]); + }); + + it('should collect element names while skipping one level of children', () => { + const root = ( +
+
+

Title

+

Text

+
+
+
+

Title

+ Text +
+
+ ); + + const { names } = traverseElementTree({ + root, + discoverers: [ + el => + Children.toArray(el.props.children).flatMap(child => + isValidElement(child) ? child?.props?.children : [], + ), + ], + collectors: { + names: createCollector(Array(), (acc, el) => { + if (typeof el.type === 'string') { + acc.push(el.type); + } + }), + }, + }); + + expect(names).toEqual(['main', 'h1', 'p', 'h2', 'span']); + }); +}); diff --git a/packages/core-api/src/routing/discovery.tsx b/packages/core-api/src/extensions/traversal.ts similarity index 68% rename from packages/core-api/src/routing/discovery.tsx rename to packages/core-api/src/extensions/traversal.ts index 0189ae3a4e..a1fbdab25a 100644 --- a/packages/core-api/src/routing/discovery.tsx +++ b/packages/core-api/src/extensions/traversal.ts @@ -15,12 +15,10 @@ */ import { isValidElement, ReactNode, ReactElement, Children } from 'react'; -import { RouteRef } from './types'; -import { getComponentData } from '../extensions'; -type Discoverer = (element: ReactElement) => ReactNode; +export type Discoverer = (element: ReactElement) => ReactNode; -type Collector = () => { +export type Collector = () => { accumulator: Result; visit( accumulator: Result, @@ -121,6 +119,13 @@ export function traverseElementTree(options: { ) as Results; } +export function createCollector( + initialResult: Result, + visit: ReturnType>['visit'], +): Collector { + return () => ({ accumulator: initialResult, visit }); +} + export function childDiscoverer(element: ReactElement): ReactNode { return element.props?.children; } @@ -131,72 +136,3 @@ export function routeElementDiscoverer(element: ReactElement): ReactNode { } return undefined; } - -function createCollector( - initialResult: Result, - visit: ReturnType>['visit'], -): Collector { - return () => ({ accumulator: initialResult, visit }); -} - -export const routeCollector = createCollector( - new Map(), - (acc, node, parent) => { - if (parent.props.element === node) { - return; - } - - const path: string | undefined = node.props?.path; - const element: ReactNode = node.props?.element; - - const routeRef = getComponentData(node, 'core.mountPoint'); - if (routeRef) { - if (!path) { - throw new Error('Mounted routable extension must have a path'); - } - acc.set(routeRef, path); - } else if (isValidElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - if (elementRouteRef) { - if (!path) { - throw new Error('Route element must have a path'); - } - acc.set(elementRouteRef, path); - } - } - }, -); - -export const routeParentCollector = createCollector( - new Map(), - (acc, node, parent, parentRouteRef?: RouteRef) => { - if (parent.props.element === node) { - return parentRouteRef; - } - - const element: ReactNode = node.props?.element; - - let nextParent = parentRouteRef; - - const routeRef = getComponentData(node, 'core.mountPoint'); - if (routeRef) { - acc.set(routeRef, parentRouteRef); - nextParent = routeRef; - } else if (isValidElement(element)) { - const elementRouteRef = getComponentData( - element, - 'core.mountPoint', - ); - - if (elementRouteRef) { - acc.set(elementRouteRef, parentRouteRef); - nextParent = elementRouteRef; - } - } - - return nextParent; - }, -); diff --git a/packages/core-api/src/plugin/collection.test.tsx b/packages/core-api/src/plugin/collectors.test.tsx similarity index 84% rename from packages/core-api/src/plugin/collection.test.tsx rename to packages/core-api/src/plugin/collectors.test.tsx index fd94303ef4..f040cfd433 100644 --- a/packages/core-api/src/plugin/collection.test.tsx +++ b/packages/core-api/src/plugin/collectors.test.tsx @@ -16,13 +16,18 @@ import React, { PropsWithChildren } from 'react'; import { createRouteRef } from '../routing'; -import { createPlugin } from '../plugin'; +import { createPlugin } from './Plugin'; import { createRoutableExtension, createComponentExtension, } from '../extensions'; import { MemoryRouter, Routes, Route } from 'react-router-dom'; -import { collectPlugins } from './collection'; +import { + traverseElementTree, + childDiscoverer, + routeElementDiscoverer, +} from '../extensions/traversal'; +import { pluginCollector } from './collectors'; const mockConfig = () => ({ path: '/foo', title: 'Foo' }); const MockComponent = ({ children }: PropsWithChildren<{}>) => <>{children}; @@ -80,6 +85,14 @@ describe('collection', () => { ); - expect(collectPlugins(root)).toEqual(new Set([pluginA, pluginB, pluginC])); + const { plugins } = traverseElementTree({ + root, + discoverers: [childDiscoverer, routeElementDiscoverer], + collectors: { + plugins: pluginCollector, + }, + }); + + expect(plugins).toEqual(new Set([pluginA, pluginB, pluginC])); }); }); diff --git a/packages/core-api/src/plugin/collection.ts b/packages/core-api/src/plugin/collectors.ts similarity index 58% rename from packages/core-api/src/plugin/collection.ts rename to packages/core-api/src/plugin/collectors.ts index 0811f66445..a222746f9e 100644 --- a/packages/core-api/src/plugin/collection.ts +++ b/packages/core-api/src/plugin/collectors.ts @@ -29,44 +29,16 @@ * limitations under the License. */ -import React, { isValidElement, ReactNode } from 'react'; import { BackstagePlugin } from './types'; import { getComponentData } from '../extensions'; +import { createCollector } from '../extensions/traversal'; -export const collectPlugins = (tree: ReactNode) => { - const plugins = new Set(); - - const nodes = [tree]; - - while (nodes.length !== 0) { - const node = nodes.shift(); - if (!isIterableElement(node)) { - continue; +export const pluginCollector = createCollector( + new Set(), + (acc, node) => { + const plugin = getComponentData(node, 'core.plugin'); + if (plugin) { + acc.add(plugin); } - - React.Children.forEach(node, child => { - if (!isIterableElement(child)) { - return; - } - - const { element, children } = child.props as { - element?: ReactNode; - children?: ReactNode; - }; - const plugin = getComponentData(child, 'core.plugin'); - if (plugin) { - plugins.add(plugin); - } - if (isIterableElement(element)) { - nodes.push(element); - } - nodes.push(children); - }); - } - - return plugins; -}; - -function isIterableElement(node: ReactNode): node is JSX.Element { - return isValidElement(node) || Array.isArray(node); -} + }, +); diff --git a/packages/core-api/src/routing/discovery.test.tsx b/packages/core-api/src/routing/collectors.test.tsx similarity index 98% rename from packages/core-api/src/routing/discovery.test.tsx rename to packages/core-api/src/routing/collectors.test.tsx index c702811529..c2eefa7e82 100644 --- a/packages/core-api/src/routing/discovery.test.tsx +++ b/packages/core-api/src/routing/collectors.test.tsx @@ -15,13 +15,13 @@ */ import React, { PropsWithChildren } from 'react'; +import { routeCollector, routeParentCollector } from './collectors'; + import { traverseElementTree, childDiscoverer, routeElementDiscoverer, - routeCollector, - routeParentCollector, -} from './discovery'; +} from '../extensions/traversal'; import { createRouteRef } from './RouteRef'; import { createPlugin } from '../plugin'; import { createRoutableExtension } from '../extensions'; diff --git a/packages/core-api/src/routing/collectors.tsx b/packages/core-api/src/routing/collectors.tsx new file mode 100644 index 0000000000..fab0cbe112 --- /dev/null +++ b/packages/core-api/src/routing/collectors.tsx @@ -0,0 +1,82 @@ +/* + * Copyright 2020 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 { isValidElement, ReactNode } from 'react'; +import { RouteRef } from '../routing/types'; +import { getComponentData } from '../extensions'; +import { createCollector } from '../extensions/traversal'; + +export const routeCollector = createCollector( + new Map(), + (acc, node, parent) => { + if (parent.props.element === node) { + return; + } + + const path: string | undefined = node.props?.path; + const element: ReactNode = node.props?.element; + + const routeRef = getComponentData(node, 'core.mountPoint'); + if (routeRef) { + if (!path) { + throw new Error('Mounted routable extension must have a path'); + } + acc.set(routeRef, path); + } else if (isValidElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); + if (elementRouteRef) { + if (!path) { + throw new Error('Route element must have a path'); + } + acc.set(elementRouteRef, path); + } + } + }, +); + +export const routeParentCollector = createCollector( + new Map(), + (acc, node, parent, parentRouteRef?: RouteRef) => { + if (parent.props.element === node) { + return parentRouteRef; + } + + const element: ReactNode = node.props?.element; + + let nextParent = parentRouteRef; + + const routeRef = getComponentData(node, 'core.mountPoint'); + if (routeRef) { + acc.set(routeRef, parentRouteRef); + nextParent = routeRef; + } else if (isValidElement(element)) { + const elementRouteRef = getComponentData( + element, + 'core.mountPoint', + ); + + if (elementRouteRef) { + acc.set(elementRouteRef, parentRouteRef); + nextParent = elementRouteRef; + } + } + + return nextParent; + }, +);