-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(Schematics): use ofType operator function instead of Actions#ofType #1154
feat(Schematics): use ofType operator function instead of Actions#ofType #1154
Conversation
); | ||
expect(content).toMatch(/export class FooEffects/); | ||
expect(content).toMatch( | ||
/effect\$ = this\.actions\$.pipe\(ofType<LoadFoos>\(FooActionTypes\.LoadFoos\)\);/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename effects$
to loadFoos$
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your comment!
I renamed "effect$" to "loadFoos$".
It's more suitable in this case.
@@ -37,7 +37,7 @@ describe('Effect Schematic', () => { | |||
appTree = createWorkspace(schematicRunner, appTree); | |||
}); | |||
|
|||
it('should create an effect', () => { | |||
it('should create an effect file and its spec file', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be should create an effect with a spec file
.
@@ -50,6 +50,44 @@ describe('Effect Schematic', () => { | |||
).toBeGreaterThanOrEqual(0); | |||
}); | |||
|
|||
it('should create an effect class which has a "loadFoos$" property if feature is set', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be should create an effect that describes a source of actions within a feature
.
And maybe move the test a bit down? (after the default tests)
); | ||
}); | ||
|
||
it('should create an effect class which does not have a "loadFoos$" property if root is set', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be should create an effect that does not define a source of actions within the root
.
And maybe move the test a bit down? (after the default tests)
@tdeschryver |
@Effect() | ||
effect$ = this.actions$.ofType(<%= classify(name) %>ActionTypes.Load<%= classify(name) %>s); | ||
loadFoos$ = this.actions$.pipe(ofType<Load<%= classify(name) %>s>(<%= classify(name) %>ActionTypes.Load<%= classify(name) %>s)); | ||
<% } %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the <Load<%= classify(name) %>s>
beec706
to
851bf21
Compare
@brandonroberts |
I suppose that we should use ofType operator function instead of Actions#ofType in generated files by schematics.
Please consider this change, thank you!