From c863342e7ff6feb5ec68b68f6ca7f54e51836ae4 Mon Sep 17 00:00:00 2001 From: Mark berry Date: Fri, 24 Nov 2023 02:53:03 -0600 Subject: [PATCH 1/2] Add random attribute to bootstrapped selector --- code/frameworks/angular/package.json | 1 + .../client/angular-beta/AbstractRenderer.ts | 23 ++++++++-- .../angular-beta/RendererFactory.test.ts | 46 ++++++++++++++++++- code/yarn.lock | 1 + 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/code/frameworks/angular/package.json b/code/frameworks/angular/package.json index e2eaf1b7d8ab..6f34efa558e6 100644 --- a/code/frameworks/angular/package.json +++ b/code/frameworks/angular/package.json @@ -56,6 +56,7 @@ "@types/semver": "^7.3.4", "@types/webpack-env": "^1.18.0", "find-up": "^5.0.0", + "nanoid": "^4.0.2", "read-pkg-up": "^7.0.1", "semver": "^7.3.7", "telejson": "^7.2.0", diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index f83fd00dde84..43df332f82ab 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -3,7 +3,8 @@ import { bootstrapApplication } from '@angular/platform-browser'; import { BehaviorSubject, Subject } from 'rxjs'; import { stringify } from 'telejson'; -import { ICollection, Parameters, StoryFnAngularReturnType } from '../types'; +import { nanoid } from 'nanoid'; +import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; import { componentNgModules } from './StorybookWrapperComponent'; @@ -16,6 +17,8 @@ type StoryRenderInfo = { const applicationRefs = new Map(); +export const STORY_UID_ATTRIBUTE = 'data-sb-story-uid'; + export abstract class AbstractRenderer { /** * Wait and destroy the platform @@ -122,10 +125,15 @@ export abstract class AbstractRenderer { const analyzedMetadata = new PropertyExtractor(storyFnAngular.moduleMetadata, component); + const componentSelector = + targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE) !== null + ? `${targetSelector}[${targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE)}]` + : targetSelector; + const application = getApplication({ storyFnAngular, component, - targetSelector, + targetSelector: componentSelector, analyzedMetadata, }); @@ -161,11 +169,18 @@ export abstract class AbstractRenderer { return storyIdIsInvalidHtmlTagName ? `sb-${id.replace(invalidHtmlTag, '')}-component` : id; } + /** + * Adds DOM element that angular will use as bootstrap component. + */ protected initAngularRootElement(targetDOMNode: HTMLElement, targetSelector: string) { - // Adds DOM element that angular will use as bootstrap component // eslint-disable-next-line no-param-reassign targetDOMNode.innerHTML = ''; - targetDOMNode.appendChild(document.createElement(targetSelector)); + + targetDOMNode.setAttribute(STORY_UID_ATTRIBUTE, `${targetDOMNode.id}-${nanoid(10)}`); + const element = document.createElement(targetSelector); + element.toggleAttribute(targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE), true); + + targetDOMNode.appendChild(element); } private fullRendererRequired({ diff --git a/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts b/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts index 0dc51d15eb6c..8a543b3df201 100644 --- a/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts +++ b/code/frameworks/angular/src/client/angular-beta/RendererFactory.test.ts @@ -13,11 +13,13 @@ describe('RendererFactory', () => { let rendererFactory: RendererFactory; let rootTargetDOMNode: HTMLElement; let rootDocstargetDOMNode: HTMLElement; + let storyInDocstargetDOMNode: HTMLElement; beforeEach(async () => { rendererFactory = new RendererFactory(); document.body.innerHTML = - '
'; + '
' + + '
'; rootTargetDOMNode = global.document.getElementById('storybook-root'); rootDocstargetDOMNode = global.document.getElementById('root-docs'); (platformBrowserDynamic as any).mockImplementation(platformBrowserDynamicTesting); @@ -180,5 +182,47 @@ describe('RendererFactory', () => { const render = await rendererFactory.getRendererInstance(rootDocstargetDOMNode); expect(render).toBeInstanceOf(DocsRenderer); }); + + describe('when multiple story for the same component', () => { + it('should render both stories', async () => { + @Component({ selector: 'foo', template: '🦊' }) + class FooComponent {} + + const render = await rendererFactory.getRendererInstance( + global.document.getElementById('storybook-docs') + ); + + const targetDOMNode1 = global.document.createElement('div'); + targetDOMNode1.id = 'story-1'; + global.document.getElementById('storybook-docs').appendChild(targetDOMNode1); + await render?.render({ + storyFnAngular: { + props: {}, + }, + forced: false, + component: FooComponent, + targetDOMNode: targetDOMNode1, + }); + + const targetDOMNode2 = global.document.createElement('div'); + targetDOMNode2.id = 'story-1'; + global.document.getElementById('storybook-docs').appendChild(targetDOMNode2); + await render?.render({ + storyFnAngular: { + props: {}, + }, + forced: false, + component: FooComponent, + targetDOMNode: targetDOMNode2, + }); + + expect(global.document.querySelectorAll('#story-1 > story-1')[0].innerHTML).toBe( + '🦊' + ); + expect(global.document.querySelectorAll('#story-1 > story-1')[1].innerHTML).toBe( + '🦊' + ); + }); + }); }); }); diff --git a/code/yarn.lock b/code/yarn.lock index 88ea95a5e552..656763a90076 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -5951,6 +5951,7 @@ __metadata: jest: "npm:^29.7.0" jest-preset-angular: "npm:^13.0.1" jest-specific-snapshot: "npm:^8.0.0" + nanoid: "npm:^4.0.2" read-pkg-up: "npm:^7.0.1" semver: "npm:^7.3.7" telejson: "npm:^7.2.0" From 413f5792a0cc0ed869ea5da9a7f4d8489ddeacef Mon Sep 17 00:00:00 2001 From: Mark berry Date: Fri, 1 Dec 2023 22:22:23 -0600 Subject: [PATCH 2/2] Refactor story's unique attribute to be more predictable --- code/frameworks/angular/package.json | 1 - .../client/angular-beta/AbstractRenderer.ts | 25 ++++++----- .../src/client/angular-beta/DocsRenderer.ts | 13 +++++- .../src/client/angular-beta/utils/StoryUID.ts | 43 +++++++++++++++++++ code/yarn.lock | 1 - 5 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 code/frameworks/angular/src/client/angular-beta/utils/StoryUID.ts diff --git a/code/frameworks/angular/package.json b/code/frameworks/angular/package.json index b6d2ebc733e4..bdc500c756e2 100644 --- a/code/frameworks/angular/package.json +++ b/code/frameworks/angular/package.json @@ -56,7 +56,6 @@ "@types/semver": "^7.3.4", "@types/webpack-env": "^1.18.0", "find-up": "^5.0.0", - "nanoid": "^4.0.2", "read-pkg-up": "^7.0.1", "semver": "^7.3.7", "telejson": "^7.2.0", diff --git a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts index 43df332f82ab..ec5c2ac7ba52 100644 --- a/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/AbstractRenderer.ts @@ -3,7 +3,7 @@ import { bootstrapApplication } from '@angular/platform-browser'; import { BehaviorSubject, Subject } from 'rxjs'; import { stringify } from 'telejson'; -import { nanoid } from 'nanoid'; + import { ICollection, StoryFnAngularReturnType } from '../types'; import { getApplication } from './StorybookModule'; import { storyPropsProvider } from './StorybookProvider'; @@ -17,6 +17,12 @@ type StoryRenderInfo = { const applicationRefs = new Map(); +/** + * Attribute name for the story UID that may be written to the targetDOMNode. + * + * If a target DOM node has a story UID attribute, it will be used as part of + * the selector for the Angular component. + */ export const STORY_UID_ATTRIBUTE = 'data-sb-story-uid'; export abstract class AbstractRenderer { @@ -125,10 +131,12 @@ export abstract class AbstractRenderer { const analyzedMetadata = new PropertyExtractor(storyFnAngular.moduleMetadata, component); - const componentSelector = - targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE) !== null - ? `${targetSelector}[${targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE)}]` - : targetSelector; + const storyUid = targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE); + const componentSelector = storyUid !== null ? `${targetSelector}[${storyUid}]` : targetSelector; + if (storyUid !== null) { + const element = targetDOMNode.querySelector(targetSelector); + element.toggleAttribute(storyUid, true); + } const application = getApplication({ storyFnAngular, @@ -175,12 +183,7 @@ export abstract class AbstractRenderer { protected initAngularRootElement(targetDOMNode: HTMLElement, targetSelector: string) { // eslint-disable-next-line no-param-reassign targetDOMNode.innerHTML = ''; - - targetDOMNode.setAttribute(STORY_UID_ATTRIBUTE, `${targetDOMNode.id}-${nanoid(10)}`); - const element = document.createElement(targetSelector); - element.toggleAttribute(targetDOMNode.getAttribute(STORY_UID_ATTRIBUTE), true); - - targetDOMNode.appendChild(element); + targetDOMNode.appendChild(document.createElement(targetSelector)); } private fullRendererRequired({ diff --git a/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts b/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts index d51573376fcb..9b7629854626 100644 --- a/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts +++ b/code/frameworks/angular/src/client/angular-beta/DocsRenderer.ts @@ -1,6 +1,8 @@ import { addons } from '@storybook/preview-api'; import { DOCS_RENDERED, STORY_CHANGED } from '@storybook/core-events'; -import { AbstractRenderer } from './AbstractRenderer'; + +import { getNextStoryUID } from './utils/StoryUID'; +import { AbstractRenderer, STORY_UID_ATTRIBUTE } from './AbstractRenderer'; import { StoryFnAngularReturnType, Parameters } from '../types'; export class DocsRenderer extends AbstractRenderer { @@ -45,4 +47,13 @@ export class DocsRenderer extends AbstractRenderer { async afterFullRender(): Promise { await AbstractRenderer.resetCompiledComponents(); } + + protected override initAngularRootElement( + targetDOMNode: HTMLElement, + targetSelector: string + ): void { + super.initAngularRootElement(targetDOMNode, targetSelector); + + targetDOMNode.setAttribute(STORY_UID_ATTRIBUTE, getNextStoryUID(targetDOMNode.id)); + } } diff --git a/code/frameworks/angular/src/client/angular-beta/utils/StoryUID.ts b/code/frameworks/angular/src/client/angular-beta/utils/StoryUID.ts new file mode 100644 index 000000000000..131ebda1adf9 --- /dev/null +++ b/code/frameworks/angular/src/client/angular-beta/utils/StoryUID.ts @@ -0,0 +1,43 @@ +/** + * Count of stories for each storyId. + */ +const storyCounts = new Map(); + +/** + * Increments the count for a storyId and returns the next UID. + * + * When a story is bootstrapped, the storyId is used as the element tag. That + * becomes an issue when a story is rendered multiple times in the same docs + * page. This function returns a UID that is appended to the storyId to make + * it unique. + * + * @param storyId id of a story + * @returns uid of a story + */ +export const getNextStoryUID = (storyId: string): string => { + if (!storyCounts.has(storyId)) { + storyCounts.set(storyId, -1); + } + + const count = storyCounts.get(storyId) + 1; + storyCounts.set(storyId, count); + return `${storyId}-${count}`; +}; + +/** + * Clears the storyId counts. + * + * Can be useful for testing, where you need predictable increments, without + * reloading the global state. + * + * If onlyStoryId is provided, only that storyId is cleared. + * + * @param onlyStoryId id of a story + */ +export const clearStoryUIDs = (onlyStoryId?: string): void => { + if (onlyStoryId !== undefined && onlyStoryId !== null) { + storyCounts.delete(onlyStoryId); + } else { + storyCounts.clear(); + } +}; diff --git a/code/yarn.lock b/code/yarn.lock index 3da7817e1669..c5118b9cd016 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -5156,7 +5156,6 @@ __metadata: jest: "npm:^29.7.0" jest-preset-angular: "npm:^13.0.1" jest-specific-snapshot: "npm:^8.0.0" - nanoid: "npm:^4.0.2" read-pkg-up: "npm:^7.0.1" semver: "npm:^7.3.7" telejson: "npm:^7.2.0"