Skip to content

Commit

Permalink
Merge pull request #21059 from sheriffMoose/angular/di
Browse files Browse the repository at this point in the history
fix(angular): provide constructor dependencies
  • Loading branch information
valentinpalkovic authored Feb 20, 2023
2 parents 5ecc066 + f076f21 commit 4d0355f
Show file tree
Hide file tree
Showing 9 changed files with 242 additions and 276 deletions.
1 change: 1 addition & 0 deletions code/frameworks/angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"@angular/forms": "^15.1.1",
"@angular/platform-browser": "^15.1.1",
"@angular/platform-browser-dynamic": "^15.1.1",
"@ngxs/store": "^3.7.6",
"@types/rimraf": "^3.0.2",
"@types/tmp": "^0.2.3",
"cross-spawn": "^7.0.3",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { ApplicationRef, enableProdMode, NgModule } from '@angular/core';
import {
ApplicationRef,
enableProdMode,
importProvidersFrom,
isStandalone,
NgModule,
} from '@angular/core';
import { bootstrapApplication } from '@angular/platform-browser';

import { BehaviorSubject, Subject } from 'rxjs';
Expand All @@ -7,7 +13,7 @@ import { ICollection, Parameters, StoryFnAngularReturnType } from '../types';
import { getApplication } from './StorybookModule';
import { storyPropsProvider } from './StorybookProvider';
import { componentNgModules } from './StorybookWrapperComponent';
import { extractSingletons } from './utils/PropertyExtractor';
import { PropertyExtractor } from './utils/PropertyExtractor';

type StoryRenderInfo = {
storyFnAngular: StoryFnAngularReturnType;
Expand Down Expand Up @@ -120,11 +126,16 @@ export abstract class AbstractRenderer {

this.initAngularRootElement(targetDOMNode, targetSelector);

const analyzedMetadata = new PropertyExtractor(storyFnAngular.moduleMetadata, component);
const providers = [
// Providers for BrowserAnimations & NoopAnimationsModule
extractSingletons(storyFnAngular.moduleMetadata),
analyzedMetadata.singletons,
importProvidersFrom(
...analyzedMetadata.imports.filter((imported) => !isStandalone(imported))
),
analyzedMetadata.providers,
storyPropsProvider(newStoryProps$),
];
].filter(Boolean);

const application = getApplication({ storyFnAngular, component, targetSelector });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { map, skip } from 'rxjs/operators';
import { ICollection, NgModuleMetadata } from '../types';
import { STORY_PROPS } from './StorybookProvider';
import { ComponentInputsOutputs, getComponentInputsOutputs } from './utils/NgComponentAnalyzer';
import { extractDeclarations, extractImports, extractProviders } from './utils/PropertyExtractor';
import { PropertyExtractor } from './utils/PropertyExtractor';

const getNonInputsOutputsProps = (
ngComponentInputsOutputs: ComponentInputsOutputs,
Expand Down Expand Up @@ -52,9 +52,8 @@ export const createStorybookWrapperComponent = (
// storyComponent was not provided.
const viewChildSelector = storyComponent ?? '__storybook-noop';

const imports = extractImports(moduleMetadata, storyComponent);
const declarations = extractDeclarations(moduleMetadata, storyComponent);
const providers = extractProviders(moduleMetadata);
const analyzedMetadata = new PropertyExtractor(moduleMetadata, storyComponent);
const { imports, declarations, providers } = analyzedMetadata;

// Only create a new module if it doesn't already exist
// This is to prevent the module from being recreated on every story change
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,6 @@
import { CommonModule } from '@angular/common';
import { HttpClientModule } from '@angular/common/http';
import { NgModule } from '@angular/core';
import { BrowserModule } from '@angular/platform-browser';
import { BrowserAnimationsModule } from '@angular/platform-browser/animations';

@NgModule({
imports: [
//
BrowserModule,
BrowserAnimationsModule,
],
})
export class WithAnimationsModule {}

@NgModule({
imports: [CommonModule, HttpClientModule],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,9 @@ import {
provideAnimations,
provideNoopAnimations,
} from '@angular/platform-browser/animations';
import { HttpClientModule } from '@angular/common/http';
import {
analyzeMetadata,
extractDeclarations,
extractImports,
extractProviders,
extractSingletons,
REMOVED_MODULES,
} from './PropertyExtractor';
import { WithAnimationsModule, WithOfficialModule } from '../__testfixtures__/test.module';
import { NgModuleMetadata } from '../../types';
import { PropertyExtractor, REMOVED_MODULES } from './PropertyExtractor';
import { WithOfficialModule } from '../__testfixtures__/test.module';

const TEST_TOKEN = new InjectionToken('testToken');
const TestTokenProvider = { provide: TEST_TOKEN, useValue: 123 };
Expand All @@ -31,14 +24,34 @@ const TestModuleWithImportsAndProviders = NgModule({
providers: [TestTokenProvider],
})(class {});

const analyzeMetadata = (metadata: NgModuleMetadata, component?: any) => {
return new PropertyExtractor(metadata, component);
};
const extractImports = (metadata: NgModuleMetadata, component?: any) => {
const { imports } = new PropertyExtractor(metadata, component);
return imports;
};
const extractDeclarations = (metadata: NgModuleMetadata, component?: any) => {
const { declarations } = new PropertyExtractor(metadata, component);
return declarations;
};
const extractProviders = (metadata: NgModuleMetadata, component?: any) => {
const { providers } = new PropertyExtractor(metadata, component);
return providers;
};
const extractSingletons = (metadata: NgModuleMetadata, component?: any) => {
const { singletons } = new PropertyExtractor(metadata, component);
return singletons;
};

describe('PropertyExtractor', () => {
describe('analyzeMetadata', () => {
it('should remove BrowserModule', () => {
const metadata = {
imports: [BrowserModule],
};
const { imports, providers, singletons } = analyzeMetadata(metadata);
expect(imports.flat(Number.MAX_VALUE)).toEqual([]);
expect(imports.flat(Number.MAX_VALUE)).toEqual([CommonModule]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([]);
expect(singletons.flat(Number.MAX_VALUE)).toEqual([]);
});
Expand All @@ -48,7 +61,7 @@ describe('PropertyExtractor', () => {
imports: [BrowserAnimationsModule],
};
const { imports, providers, singletons } = analyzeMetadata(metadata);
expect(imports.flat(Number.MAX_VALUE)).toEqual([]);
expect(imports.flat(Number.MAX_VALUE)).toEqual([CommonModule]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([]);
expect(singletons.flat(Number.MAX_VALUE)).toEqual(provideAnimations());
});
Expand All @@ -58,20 +71,18 @@ describe('PropertyExtractor', () => {
imports: [NoopAnimationsModule],
};
const { imports, providers, singletons } = analyzeMetadata(metadata);
expect(imports.flat(Number.MAX_VALUE)).toEqual([]);
expect(imports.flat(Number.MAX_VALUE)).toEqual([CommonModule]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([]);
expect(singletons.flat(Number.MAX_VALUE)).toEqual(provideNoopAnimations());
});

it('should remove Browser/Animations modules recursively', () => {
const metadata = {
imports: [WithAnimationsModule],
imports: [BrowserAnimationsModule, BrowserModule],
};
const { imports, providers, singletons } = analyzeMetadata(metadata);
expect(imports.flat(Number.MAX_VALUE)).toEqual([]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([
{ provide: REMOVED_MODULES, useValue: WithAnimationsModule, multi: true },
]);
expect(imports.flat(Number.MAX_VALUE)).toEqual([CommonModule]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([]);
expect(singletons.flat(Number.MAX_VALUE)).toEqual(provideAnimations());
});

Expand All @@ -80,18 +91,16 @@ describe('PropertyExtractor', () => {
imports: [WithOfficialModule],
};
const { imports, providers, singletons } = analyzeMetadata(metadata);
expect(imports.flat(Number.MAX_VALUE)).toEqual([CommonModule, HttpClientModule]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([
{ provide: REMOVED_MODULES, useValue: WithOfficialModule, multi: true },
]);
expect(imports.flat(Number.MAX_VALUE)).toEqual([CommonModule, WithOfficialModule]);
expect(providers.flat(Number.MAX_VALUE)).toEqual([]);
expect(singletons.flat(Number.MAX_VALUE)).toEqual([]);
});
});

describe('extractImports', () => {
it('should return Angular official modules', () => {
const imports = extractImports({ imports: [TestModuleWithImportsAndProviders] });
expect(imports).toEqual([CommonModule]);
expect(imports).toEqual([CommonModule, TestModuleWithImportsAndProviders]);
});

it('should return standalone components', () => {
Expand All @@ -101,17 +110,11 @@ describe('PropertyExtractor', () => {
},
StandaloneTestComponent
);
expect(imports).toEqual([CommonModule, StandaloneTestComponent]);
});

it('should not return any regular modules (they get destructured)', () => {
const imports = extractImports({
imports: [
TestModuleWithDeclarations,
{ ngModule: TestModuleWithImportsAndProviders, providers: [] },
],
});
expect(imports).toEqual([CommonModule]);
expect(imports).toEqual([
CommonModule,
TestModuleWithImportsAndProviders,
StandaloneTestComponent,
]);
});
});

Expand All @@ -120,44 +123,6 @@ describe('PropertyExtractor', () => {
const declarations = extractDeclarations({ declarations: [TestComponent1] }, TestComponent2);
expect(declarations).toEqual([TestComponent1, TestComponent2]);
});

it('should ignore `storyComponent` if it is already declared', () => {
const declarations = extractDeclarations(
{
imports: [TestModuleWithImportsAndProviders],
declarations: [TestComponent2, StandaloneTestComponent, TestDirective],
},
TestComponent1
);
expect(declarations).toEqual([
TestComponent1,
TestComponent2,
StandaloneTestComponent,
TestDirective,
]);
});

it('should ignore `storyComponent` if it is standalone', () => {
const declarations = extractDeclarations(
{
imports: [TestModuleWithImportsAndProviders],
declarations: [TestComponent1, TestComponent2, TestDirective],
},
StandaloneTestComponent
);
expect(declarations).toEqual([TestComponent1, TestComponent2, TestDirective]);
});

it('should ignore `storyComponent` if it is not a component/directive/pipe', () => {
const declarations = extractDeclarations(
{
imports: [TestModuleWithImportsAndProviders],
declarations: [TestComponent1, TestComponent2, StandaloneTestComponent],
},
TestService
);
expect(declarations).toEqual([TestComponent1, TestComponent2, StandaloneTestComponent]);
});
});

describe('extractProviders', () => {
Expand All @@ -168,39 +133,6 @@ describe('PropertyExtractor', () => {
expect(providers).toEqual([TestService]);
});

it('should return an array of providers extracted from ModuleWithProviders', () => {
const providers = extractProviders({
imports: [{ ngModule: TestModuleWithImportsAndProviders, providers: [TestService] }],
});
expect(providers).toEqual([
{ provide: TEST_TOKEN, useValue: 123 },
{ provide: REMOVED_MODULES, useValue: TestModuleWithDeclarations, multi: true },
TestService,
{ provide: REMOVED_MODULES, useValue: TestModuleWithImportsAndProviders, multi: true },
]);
});

it('should return an array of unique providers', () => {
const providers = extractProviders({
imports: [{ ngModule: TestModuleWithImportsAndProviders, providers: [TestService] }],
providers: [TestService, { provide: TEST_TOKEN, useValue: 123 }],
});
expect(providers).toEqual([
{ provide: TEST_TOKEN, useValue: 123 },
{ provide: REMOVED_MODULES, useValue: TestModuleWithDeclarations, multi: true },
TestService,
{
provide: new InjectionToken('testToken'),
useValue: 123,
},
{
provide: REMOVED_MODULES,
useValue: TestModuleWithImportsAndProviders,
multi: true,
},
]);
});

it('should return an array of singletons extracted', () => {
const singeltons = extractSingletons({
imports: [BrowserAnimationsModule],
Expand Down
Loading

0 comments on commit 4d0355f

Please sign in to comment.