From 7370887e4d21de1345a307f9c88876fa72c64577 Mon Sep 17 00:00:00 2001 From: ThibaudAv Date: Thu, 26 Nov 2020 14:38:09 +0100 Subject: [PATCH] refactor: rework story's props association with the component's properties - allow story with component to override local properties - handles Input with `bindingPropertyName` --- .../ComponentClassFromStoryComponent.ts | 107 +++++--- .../angular/RenderNgAppService.test.ts | 242 ++++++++++++++++-- .../preview/angular/RenderNgAppService.ts | 69 +++-- 3 files changed, 323 insertions(+), 95 deletions(-) diff --git a/app/angular/src/client/preview/angular/ComponentClassFromStoryComponent.ts b/app/angular/src/client/preview/angular/ComponentClassFromStoryComponent.ts index d00753adde83..5c04b195f75c 100644 --- a/app/angular/src/client/preview/angular/ComponentClassFromStoryComponent.ts +++ b/app/angular/src/client/preview/angular/ComponentClassFromStoryComponent.ts @@ -14,35 +14,28 @@ import { map, skip } from 'rxjs/operators'; import { ICollection } from '../types'; import { STORY_PROPS } from './app.token'; +import { + ComponentInputsOutputs, + getComponentDecoratorMetadata, + getComponentInputsOutputs, +} from './NgComponentAnalyzer'; import { RenderNgAppService } from './RenderNgAppService'; -const findComponentDecoratorMetadata = (component: any) => { - const decoratorKey = '__annotations__'; - const decorators: any[] = - Reflect && - Reflect.getOwnPropertyDescriptor && - Reflect.getOwnPropertyDescriptor(component, decoratorKey) - ? Reflect.getOwnPropertyDescriptor(component, decoratorKey).value - : (component as any)[decoratorKey]; - - const ngComponentDecorator: Component | undefined = decorators.find( - (decorator) => decorator instanceof Component - ); - - return ngComponentDecorator; -}; - -const toInputsOutputs = (props: ICollection = {}) => { - return Object.entries(props).reduce( - (previousValue, [key, value]) => { - if (typeof value === 'function') { - return { ...previousValue, outputs: { ...previousValue.outputs, [key]: value } }; - } - - return { ...previousValue, inputs: { ...previousValue.inputs, [key]: value } }; - }, - { inputs: {}, outputs: {} } as { inputs: Record; outputs: Record } - ); +const distinctComponentInputsOutputsWithProps = ( + ngComponentInputsOutputs: ComponentInputsOutputs, + props: ICollection = {} +) => { + const inputs = ngComponentInputsOutputs.inputs + .filter((i) => i.templateName in props) + .map((i) => i.templateName); + const outputs = ngComponentInputsOutputs.outputs + .filter((o) => o.templateName in props) + .map((i) => i.templateName); + return { + inputs, + outputs, + otherProps: Object.keys(props).filter((k) => ![...inputs, ...outputs].includes(k)), + }; }; /** @@ -55,16 +48,17 @@ export const createComponentClassFromStoryComponent = ( component: any, initialProps?: ICollection ): Type => { - const ngComponentMetadata = findComponentDecoratorMetadata(component); + const ngComponentMetadata = getComponentDecoratorMetadata(component); + const ngComponentInputsOutputs = getComponentInputsOutputs(component); - const { inputs: initialInputs, outputs: initialOutputs } = toInputsOutputs(initialProps); + const { + inputs: initialInputs, + outputs: initialOutputs, + otherProps: initialOtherProps, + } = distinctComponentInputsOutputsWithProps(ngComponentInputsOutputs, initialProps); - const templateInputs = Object.keys(initialInputs) - .map((i) => `[${i}]="${i}"`) - .join(' '); - const templateOutputs = Object.keys(initialOutputs) - .map((i) => `(${i})="${i}($event)"`) - .join(' '); + const templateInputs = initialInputs.map((i) => `[${i}]="${i}"`).join(' '); + const templateOutputs = initialOutputs.map((i) => `(${i})="${i}($event)"`).join(' '); @Component({ selector: RenderNgAppService.SELECTOR_STORYBOOK_WRAPPER, @@ -91,12 +85,49 @@ export const createComponentClassFromStoryComponent = ( } ngAfterViewInit(): void { + // Initializes properties that are not Inputs | Outputs + // Allows story props to override local component properties + initialOtherProps.forEach((p) => { + (this.storyComponentElementRef as any)[p] = initialProps[p]; + }); + + // `markForCheck` the component in case this uses changeDetection: OnPush + // And then forces the `detectChanges` + this.storyComponentViewContainerRef.injector.get(ChangeDetectorRef).markForCheck(); + this.changeDetectorRef.detectChanges(); + // Once target component has been initialized, the storyProps$ observable keeps target component inputs up to date this.storyPropsSubscription = this.storyProps$ - .pipe(skip(1), map(toInputsOutputs)) - .subscribe(({ inputs }) => { + .pipe( + skip(1), + map((props) => { + // removes component output in props + const outputsKeyToRemove = ngComponentInputsOutputs.outputs.map((o) => o.templateName); + return Object.entries(props).reduce( + (prev, [key, value]) => ({ + ...prev, + ...(!outputsKeyToRemove.includes(key) && { [key]: value }), + }), + {} as ICollection + ); + }), + map((props) => { + // In case a component uses an input with `bindingPropertyName` (ex: @Input('name')) + // find the value of the local propName in the component Inputs + // otherwise use the input key + return Object.entries(props).reduce((prev, [propKey, value]) => { + const input = ngComponentInputsOutputs.inputs.find((o) => o.templateName === propKey); + + return { + ...prev, + ...(input ? { [input.propName]: value } : { [propKey]: value }), + }; + }, {} as ICollection); + }) + ) + .subscribe((props) => { // Replace inputs with new ones from props - Object.assign(this.storyComponentElementRef, inputs); + Object.assign(this.storyComponentElementRef, props); // `markForCheck` the component in case this uses changeDetection: OnPush // And then forces the `detectChanges` diff --git a/app/angular/src/client/preview/angular/RenderNgAppService.test.ts b/app/angular/src/client/preview/angular/RenderNgAppService.test.ts index 26f533cfb6e6..20d29a61d4a5 100644 --- a/app/angular/src/client/preview/angular/RenderNgAppService.test.ts +++ b/app/angular/src/client/preview/angular/RenderNgAppService.test.ts @@ -1,7 +1,23 @@ -import { Component } from '@angular/core'; +import { + AfterViewInit, + Component, + EventEmitter, + Input, + NgModule, + OnInit, + Output, + Type, +} from '@angular/core'; import { platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing'; import { platformBrowserDynamic } from '@angular/platform-browser-dynamic'; +import { + ComponentFixture, + ComponentFixtureAutoDetect, + getTestBed, + TestBed, +} from '@angular/core/testing'; +import { BehaviorSubject } from 'rxjs'; import { StoryFnAngularReturnType } from '../types'; import { RenderNgAppService } from './RenderNgAppService'; @@ -25,28 +41,216 @@ describe('RenderNgAppService', () => { expect(renderNgAppService).toBeDefined(); }); - it('should add storybook-wrapper for story template', async () => { - await renderNgAppService.render( - (): StoryFnAngularReturnType => ({ template: '🦊', props: {} }), - false - ); + describe('render', () => { + it('should add storybook-wrapper for story template', async () => { + await renderNgAppService.render( + (): StoryFnAngularReturnType => ({ + template: '🦊', + props: {}, + }), + false + ); - expect(document.body.innerHTML).toBe( - '
🦊
' - ); + expect(document.body.innerHTML).toBe( + '
🦊
' + ); + }); + + it('should add storybook-wrapper for story component', async () => { + @Component({ selector: 'foo', template: '🦊' }) + class FooComponent {} + + await renderNgAppService.render( + (): StoryFnAngularReturnType => ({ + component: FooComponent, + props: {}, + }), + false + ); + + expect(document.body.innerHTML).toBe( + '
🦊
' + ); + }); }); + describe('getNgModuleMetadata', () => { + describe('with simple component', () => { + @Component({ + selector: 'foo', + template: ` +

{{ input }}

+

{{ localPropertyName }}

+

{{ localProperty }}

+

{{ localFunction() }}

+

+

+ `, + }) + class FooComponent { + @Input() + public input: string; + + @Input('inputBindingPropertyName') + public localPropertyName: string; + + @Output() + public output = new EventEmitter(); + + @Output('outputBindingPropertyName') + public localOutput = new EventEmitter(); + + public localProperty: string; + + public localFunction = () => ''; + } + + it('should initialize inputs', async () => { + const props = { + input: 'input', + inputBindingPropertyName: 'inputBindingPropertyName', + localProperty: 'localProperty', + localFunction: () => 'localFunction', + }; + + const ngModule = renderNgAppService.getNgModuleMetadata( + { component: FooComponent, props }, + new BehaviorSubject(props) + ); + + const { fixture } = await configureTestingModule(ngModule); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('p#input').innerHTML).toEqual(props.input); + expect(fixture.nativeElement.querySelector('p#inputBindingPropertyName').innerHTML).toEqual( + props.inputBindingPropertyName + ); + expect(fixture.nativeElement.querySelector('p#localProperty').innerHTML).toEqual( + props.localProperty + ); + expect(fixture.nativeElement.querySelector('p#localFunction').innerHTML).toEqual( + props.localFunction() + ); + }); + + it('should initialize outputs', async () => { + let expectedOutputValue: string; + let expectedOutputBindingValue: string; + const props = { + output: (value: string) => { + expectedOutputValue = value; + }, + outputBindingPropertyName: (value: string) => { + expectedOutputBindingValue = value; + }, + }; - it('should add storybook-wrapper for story component', async () => { - @Component({ selector: 'foo', template: '🦊' }) - class FooComponent {} + const ngModule = renderNgAppService.getNgModuleMetadata( + { component: FooComponent, props }, + new BehaviorSubject(props) + ); - await renderNgAppService.render( - (): StoryFnAngularReturnType => ({ component: FooComponent, props: {} }), - false - ); + const { fixture } = await configureTestingModule(ngModule); + fixture.detectChanges(); - expect(document.body.innerHTML).toBe( - '
🦊
' - ); + fixture.nativeElement.querySelector('p#output').click(); + fixture.nativeElement.querySelector('p#outputBindingPropertyName').click(); + + expect(expectedOutputValue).toEqual('outputEmitted'); + expect(expectedOutputBindingValue).toEqual('outputEmitted'); + }); + + it('should change inputs if storyProps$ Subject emit', async () => { + const initialProps = { + input: 'input', + }; + const storyProps$ = new BehaviorSubject(initialProps); + + const ngModule = renderNgAppService.getNgModuleMetadata( + { component: FooComponent, props: initialProps }, + storyProps$ + ); + const { fixture } = await configureTestingModule(ngModule); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('p#input').innerHTML).toEqual( + initialProps.input + ); + expect(fixture.nativeElement.querySelector('p#inputBindingPropertyName').innerHTML).toEqual( + '' + ); + + const newProps = { + input: 'new input', + inputBindingPropertyName: 'new inputBindingPropertyName', + localProperty: 'new localProperty', + localFunction: () => 'new localFunction', + }; + storyProps$.next(newProps); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('p#input').innerHTML).toEqual(newProps.input); + expect(fixture.nativeElement.querySelector('p#inputBindingPropertyName').innerHTML).toEqual( + newProps.inputBindingPropertyName + ); + expect(fixture.nativeElement.querySelector('p#localProperty').innerHTML).toEqual( + newProps.localProperty + ); + expect(fixture.nativeElement.querySelector('p#localFunction').innerHTML).toEqual( + newProps.localFunction() + ); + }); + + it('should not override outputs if storyProps$ Subject emit', async () => { + let expectedOutputValue; + let expectedOutputBindingValue; + const initialProps = { + output: (value: string) => { + expectedOutputValue = value; + }, + outputBindingPropertyName: (value: string) => { + expectedOutputBindingValue = value; + }, + }; + const storyProps$ = new BehaviorSubject(initialProps); + + const ngModule = renderNgAppService.getNgModuleMetadata( + { component: FooComponent, props: initialProps }, + storyProps$ + ); + const { fixture } = await configureTestingModule(ngModule); + fixture.detectChanges(); + + const newProps = { + input: 'new input', + output: (value: string) => { + expectedOutputValue = 'should not be called'; + }, + outputBindingPropertyName: (value: string) => { + expectedOutputBindingValue = 'should not be called'; + }, + }; + storyProps$.next(newProps); + fixture.detectChanges(); + + fixture.nativeElement.querySelector('p#output').click(); + fixture.nativeElement.querySelector('p#outputBindingPropertyName').click(); + + expect(fixture.nativeElement.querySelector('p#input').innerHTML).toEqual(newProps.input); + expect(expectedOutputValue).toEqual('outputEmitted'); + expect(expectedOutputBindingValue).toEqual('outputEmitted'); + }); + }); }); + + async function configureTestingModule(ngModule: NgModule) { + await TestBed.configureTestingModule({ + declarations: ngModule.declarations, + providers: ngModule.providers, + }).compileComponents(); + const fixture = TestBed.createComponent(ngModule.bootstrap[0] as Type); + + return { + fixture, + }; + } }); diff --git a/app/angular/src/client/preview/angular/RenderNgAppService.ts b/app/angular/src/client/preview/angular/RenderNgAppService.ts index f188d781dc32..2fee9e2a484e 100644 --- a/app/angular/src/client/preview/angular/RenderNgAppService.ts +++ b/app/angular/src/client/preview/angular/RenderNgAppService.ts @@ -78,48 +78,41 @@ export class RenderNgAppService { this.storyProps$ = new BehaviorSubject(storyObj.props); await this.platform.bootstrapModule( - createModuleFromMetadata(getNgModuleMetadata(storyObj, this.storyProps$)) + createModuleFromMetadata(this.getNgModuleMetadata(storyObj, this.storyProps$)) ); } -} - -const getNgModuleMetadata = ( - storyFnAngular: StoryFnAngularReturnType, - storyProps$: Subject -): NgModule => { - const { component, moduleMetadata = {} } = storyFnAngular; - - const ComponentToInject = createComponentToInject(storyFnAngular); - - // Look recursively (deep) if the component is not already declared by an import module - const requiresComponentDeclaration = - component && - !getExistenceOfComponentInModules( - component, - moduleMetadata.declarations, - moduleMetadata.imports - ); - return { - declarations: [ - ...(requiresComponentDeclaration ? [component] : []), - ComponentToInject, - ...(moduleMetadata.declarations ?? []), - ], - imports: [BrowserModule, ...(moduleMetadata.imports ?? [])], - providers: [storyPropsProvider(storyProps$), ...(moduleMetadata.providers ?? [])], - entryComponents: [...(moduleMetadata.entryComponents ?? [])], - schemas: [ - // Required because`props` don't only contain Inputs/Outputs which generates a bad template for component wrapper - // This happens with the addon Controls + Doc - // The private and public properties of the component are also added in `props` (wrongly) - // FIXME : To be removed when the above behavior is corrected - NO_ERRORS_SCHEMA, - ...(moduleMetadata.schemas ?? []), - ], - bootstrap: [ComponentToInject], + public getNgModuleMetadata = ( + storyFnAngular: StoryFnAngularReturnType, + storyProps$: Subject + ): NgModule => { + const { component, moduleMetadata = {} } = storyFnAngular; + + const ComponentToInject = createComponentToInject(storyFnAngular); + + // Look recursively (deep) if the component is not already declared by an import module + const requiresComponentDeclaration = + component && + !getExistenceOfComponentInModules( + component, + moduleMetadata.declarations, + moduleMetadata.imports + ); + + return { + declarations: [ + ...(requiresComponentDeclaration ? [component] : []), + ComponentToInject, + ...(moduleMetadata.declarations ?? []), + ], + imports: [BrowserModule, ...(moduleMetadata.imports ?? [])], + providers: [storyPropsProvider(storyProps$), ...(moduleMetadata.providers ?? [])], + entryComponents: [...(moduleMetadata.entryComponents ?? [])], + schemas: [...(moduleMetadata.schemas ?? [])], + bootstrap: [ComponentToInject], + }; }; -}; +} const createModuleFromMetadata = (ngModule: NgModule) => { @NgModule(ngModule)