Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core): allow readonly arrays in NgModule imports and exports #48106

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+73 −4
Open

fix(core): allow readonly arrays in NgModule imports and exports #48106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/public-api/core/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -922,9 +922,9 @@ export interface NgModule {
declarations?: Array<Type<any> | any[]>;
// @deprecated
entryComponents?: Array<Type<any> | any[]>;
exports?: Array<Type<any> | any[]>;
exports?: Array<Type<any> | ReadonlyArray<any>>;
id?: string;
imports?: Array<Type<any> | ModuleWithProviders<{}> | any[]>;
imports?: Array<Type<any> | ModuleWithProviders<{}> | ReadonlyArray<any>>;
jit?: true;
providers?: Array<Provider | EnvironmentProviders>;
schemas?: Array<SchemaMetadata | any[]>;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/metadata/ng_module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export interface NgModule {
* ```
*
*/
imports?: Array<Type<any>|ModuleWithProviders<{}>|any[]>;
imports?: Array<Type<any>|ModuleWithProviders<{}>|ReadonlyArray<any>>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be:

Suggested change
imports?: Array<Type<any>|ModuleWithProviders<{}>|ReadonlyArray<any>>;
imports?: Array<Type<any>|ModuleWithProviders<{}>|ReadonlyArray<Type<any>>>;

in which case I think we can just do:

Suggested change
imports?: Array<Type<any>|ModuleWithProviders<{}>|ReadonlyArray<any>>;
imports?: ReadonlyArray<Type<any>|ModuleWithProviders<{}>;

as all Arrays are implicitly ReadonlyArrays.

Copy link

@csisy csisy Aug 3, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can Type<any> represent an array of types? Previously, it was valid to import an array directly, like this:

const myArr = [/*...*/];
import: [myArr]

IMO keeping the type as-is can be more descriptive about what the import accepts.


/**
* The set of components, directives, and pipes declared in this
Expand Down Expand Up @@ -168,7 +168,7 @@ export interface NgModule {
* }
* ```
*/
exports?: Array<Type<any>|any[]>;
exports?: Array<Type<any>|ReadonlyArray<any>>;

/**
* The set of components to compile when this NgModule is defined,
Expand Down
69 changes: 69 additions & 0 deletions packages/core/test/acceptance/standalone_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,75 @@ describe('standalone components, directives, and pipes', () => {
expect(fixture.nativeElement.innerHTML).toBe('<div red="true">blue</div>');
});

it('should support readonly arrays in @NgModule.imports and @NgModule.exports', () => {
@Directive({selector: '[red]', standalone: true, host: {'[attr.red]': 'true'}})
class RedIdDirective {
}

@Pipe({name: 'blue', pure: true, standalone: true})
class BluePipe implements PipeTransform {
transform() {
return 'blue';
}
}

const DirAndPipe = [RedIdDirective, BluePipe] as const;

@NgModule({
imports: [DirAndPipe],
exports: [DirAndPipe],
})
class TestNgModule {
}

@Component({
selector: 'uses-standalone',
template: `<div red>{{'' | blue}}</div>`,
})
class TestComponent {
}

const fixture = TestBed
.configureTestingModule({
declarations: [TestComponent],
imports: [TestNgModule],
})
.createComponent(TestComponent);
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toBe('<div red="true">blue</div>');
});

it('should support readonly arrays in TestBed testing module imports', () => {
@Directive({selector: '[red]', standalone: true, host: {'[attr.red]': 'true'}})
class RedIdDirective {
}

@Pipe({name: 'blue', pure: true, standalone: true})
class BluePipe implements PipeTransform {
transform() {
return 'blue';
}
}

const DirAndPipe = [RedIdDirective, BluePipe] as const;

@Component({
selector: 'uses-standalone',
template: `<div red>{{'' | blue}}</div>`,
})
class TestComponent {
}

const fixture = TestBed
.configureTestingModule({
declarations: [TestComponent],
imports: [DirAndPipe],
})
.createComponent(TestComponent);
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toBe('<div red="true">blue</div>');
});

it('should deduplicate declarations', () => {
@Component({selector: 'test-red', standalone: true, template: 'red(<ng-content></ng-content>)'})
class RedComponent {
Expand Down