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.

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

fix(schematics): correct a type of action class generated #1140

Merged

Conversation

koumatsumoto
Copy link
Contributor

@koumatsumoto koumatsumoto commented Jun 21, 2018

I used ng generate action command and found the generated file are invalid.
Please check the result I generated, thank you.

  • Result I generated
import { Action } from '@ngrx/store';

export enum FooActionTypes {
  LoadFoos = '[Foo] Load Foos'
}

export class Foo implements Action {
  readonly type = FooActionTypes.LoadFoos;
}

export type FooActions = LoadFoos;
  • Expected
import { Action } from '@ngrx/store';

export enum FooActionTypes {
  LoadFoos = '[Foo] Load Foos'
}

export class Foo implements Action {
  readonly type = FooActionTypes.LoadFoos;
}

export type FooActions = Foo; // <<< this should be Action class name!

@coveralls
Copy link

coveralls commented Jun 21, 2018

Coverage Status

Coverage increased (+0.03%) to 87.998% when pulling af01c75 on kouMatsumoto:correct-schematics-action-name into 55a0488 on ngrx:master.

@MatsumotoKou
Copy link

In advance, I suppose the generated action class name Foo should be LoadFoos.

  • Expected
import { Action } from '@ngrx/store';

export enum FooActionTypes {
  LoadFoos = '[Foo] Load Foos'
}

export class LoadFoos implements Action {
  readonly type = FooActionTypes.LoadFoos;
}

export type FooActions = LoadFoos;

@timdeschryver
Copy link
Member

@MatsumotoKou Indeed, it should be generated as LoadFoos:

import { Action } from '@ngrx/store';

export enum FooActionTypes {
  LoadFoos = '[Foo] Load Foos'
}

export class LoadFoos implements Action {
  readonly type = FooActionTypes.LoadFoos;
}

export type FooActions = LoadFoos;

@MatsumotoKou
Copy link

I will commit additionally if needed, thank you.

@MatsumotoKou
Copy link

@tdeschryver
I updated the action class name and spec files.
please review me, thank you.

@koumatsumoto koumatsumoto changed the title fix(schematics): correct an type of action class generated fix(schematics): correct a type of action class generated Jun 22, 2018
@@ -69,6 +69,32 @@ describe('Action Schematic', () => {
expect(fileContent).toMatch(/export enum FooActionTypes/);
});

it('should create a class named "LoadFoos"', () => {
Copy link
Member

Choose a reason for hiding this comment

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

This should be more like should create a class based on the provided name

expect(fileContent).toMatch(/export class LoadFoos implements Action/);
});

it('should create a type named "FooActions"', () => {
Copy link
Member

Choose a reason for hiding this comment

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

This should be more like should create the union type based on the provided name

@koumatsumoto
Copy link
Contributor Author

@brandonroberts
Thank you for pointing that out.
I revised it based on your comments.
Please continue the review, thank you!

@brandonroberts brandonroberts merged commit bbb7e8c into ngrx:master Jun 23, 2018
@brandonroberts
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants