feat(schematics): add entity generation as part of feature schematic #3850

wgd3 · 2023-04-17T18:02:26Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/ngrx/platform/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Documentation has been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

If a user wishes to create an entity as part of a new feature state, they must run multiple commands:

ng generate feature User

ng generate entity User -m app.module.ts -r user.reducer.ts

Closes #3260

What is the new behavior?

Users can now optionally generate an entity when running the feature schematic:

ng generate feature User --entity

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

This functionality does not allow for custom entity names - instead, a boolean is passed to the feature schematic. If this needs to be changed I'm happy to update this PR allowing for custom entity names.

netlify · 2023-04-17T18:02:44Z

✅ Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`8702590`
🔍 Latest deploy log	https://app.netlify.com/sites/ngrx-io/deploys/64ca82e87a043c0008b4fea7
😎 Deploy Preview	https://deploy-preview-3850--ngrx-io.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

modules/schematics/src/selector/schema.json

modules/schematics/src/feature/schema.json

modules/schematics/src/reducer/index.ts

modules/schematics/src/feature/index.ts

wgd3 · 2023-05-10T16:06:10Z

@timdeschryver I've refactored my local branch based on your recommendations - that turned out to be a much simpler change to the code base than my initial commits. Before I push my changes, I wanted to get your workflow preference:

There were enough changes between my commits and the v16 release (congrats!) that I ran git reset --hard upstream/main to start fresh and up-to-date. Then applied my changes on there. Are you/the team ok with a force-push to this branch? Or would you prefer I close this PR and open a new one?

timdeschryver · 2023-05-18T17:22:12Z

Thanks in advance @wgd3.
I'm currently a bit busy but I'll review it later this week.

commit f8407f5be9e4f2f9106957d625475fd0e6bdf403 Author: wgd3 <[email protected]> Date: Wed May 10 11:17:24 2023 -0400 added pluralization to entity actions where necessary commit 757ccfe06178ce566afcf64f9ba0515248369d80 Author: wgd3 <[email protected]> Date: Wed May 10 11:00:48 2023 -0400 updated tests to reflect files generated commit e7879737616660d693506b4ab3d7c88c90a39016 Author: wgd3 <[email protected]> Date: Wed May 10 10:36:38 2023 -0400 removed selectors schematic from feature entity, duplicate effort commit a8d8e1d426704a60e0fe154fe85bed6830b9fe3e Author: wgd3 <[email protected]> Date: Wed May 10 10:33:02 2023 -0400 got entity generation working without clobbering files

yarn.lock

wgd3 · 2023-06-28T19:18:48Z

With the new snapshots in place, I rebased and ran the tests for this PR again. One of the first things caught in the pipeline:

    -     'Add Foos': props<{ foo: Foo[] }>(),
    +     'Add Foos': props<{ foos: Foo[] }>(),
    -     'Upsert Foos': props<{ foo: Foo[] }>(),
    +     'Upsert Foos': props<{ foos: Foo[] }>(),

This came from a change in the actions template of the Entity schematic that I made in one of the older commits. I reverted that with 87b34e9.

The other test/snapshot failure was that the order of imports had changed in the generated code for Entity:

    -     StoreModule.forFeature(fromFoo.fooFeatureKey, fromFoo.reducer),
    -     EffectsModule.forFeature([FooEffects])
    +     EffectsModule.forFeature([FooEffects]),
    +     StoreModule.forFeature(fromFoo.fooFeatureKey, fromFoo.reducer)

This was remedied in 0460889 by changing the order in which schematics are run as part of the Feature schematic.

Please let me know if you have any other feedback!

brandonroberts · 2023-06-29T13:55:04Z

modules/schematics/src/feature/index.spec.ts

@@ -269,4 +271,40 @@ describe('Feature Schematic', () => {

    expect(fileContent).toMatchSnapshot();
  });
+
+  it('should create all files of a feature with an entity', async () => {


How about we use snapshot tests for this

timdeschryver

Sorry I forgot about this.
Thanks for the rebase, this gives the confidence that the current implementation is still working for the other configs.
As @brandonroberts mentioned, I agree that including snapshots for the generated files is helpfull.

modules/schematics/src/feature/index.spec.ts

Co-authored-by: Tim Deschryver <[email protected]>

wgd3 · 2023-08-02T16:23:18Z

@timdeschryver I've removed the Actions spec file as well as the Selector/Selector spec files from the code in your commit. The Entity schematic doesn't have templates for any of these files at the moment.

You mentioned that we should let the Entity schematic handle these things for now. If you/the team would prefer that selectors be generated as well, I can update the Feature schematic to run the Selector schematic every time.

Snapshots have been updated!

timdeschryver · 2023-08-05T18:07:20Z

modules/schematics/src/feature/__snapshots__/index.spec.ts.snap

+export const {
+  selectIds,
+  selectEntities,
+  selectAll,
+  selectTotal,
+} = foosFeature;


@timdeschryver I've removed the Actions spec file as well as the Selector/Selector spec files from the code in your commit. The Entity schematic doesn't have templates for any of these files at the moment.

You mentioned that we should let the Entity schematic handle these things for now. If you/the team would prefer that selectors be generated as well, I can update the Feature schematic to run the Selector schematic every time.

Snapshots have been updated!

The selectors are already included within this feature file.
If I'm missing something feel free to correct me.

timdeschryver

LGTM, thanks for the updates @wgd3 !

timdeschryver requested changes Apr 20, 2023

View reviewed changes

markostanimirovic added the Needs Cleanup Review changes needed label May 4, 2023

brandonroberts changed the base branch from master to main May 10, 2023 02:17

wgd3 force-pushed the issue-3260 branch from d9beac7 to c191c9a Compare May 24, 2023 15:28

timdeschryver reviewed May 27, 2023

View reviewed changes

yarn.lock Outdated Show resolved Hide resolved

wgd3 and others added 6 commits May 30, 2023 09:32

reverting yarn.lock

e70d23c

Merge branch 'main' into issue-3260

19d3d42

Merge branch 'main' into issue-3260

bf386a5

Merge branch 'main' into issue-3260

e6d983f

updated schematic order for feature to match snapshot

0460889

removed additional pluralization in actions template

87b34e9

brandonroberts requested changes Jun 29, 2023

View reviewed changes

brandonroberts requested a review from timdeschryver June 29, 2023 13:55

timdeschryver reviewed Jul 1, 2023

View reviewed changes

modules/schematics/src/feature/index.spec.ts Outdated Show resolved Hide resolved

wgd3 and others added 3 commits August 2, 2023 10:52

Update modules/schematics/src/feature/index.spec.ts

bd16c48

Co-authored-by: Tim Deschryver <[email protected]>

Merge branch 'main' into issue-3260

11e2d3b

updated snapshots for feature schematic

8702590

timdeschryver removed the Needs Cleanup Review changes needed label Aug 5, 2023

timdeschryver reviewed Aug 5, 2023

View reviewed changes

timdeschryver approved these changes Aug 5, 2023

View reviewed changes

brandonroberts approved these changes Aug 5, 2023

View reviewed changes

brandonroberts merged commit 19ebb0a into ngrx:main Aug 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(schematics): add entity generation as part of feature schematic #3850

feat(schematics): add entity generation as part of feature schematic #3850

wgd3 commented Apr 17, 2023

netlify bot commented Apr 17, 2023 •

edited

Loading

wgd3 commented May 10, 2023

timdeschryver commented May 18, 2023

wgd3 commented Jun 28, 2023

brandonroberts Jun 29, 2023

timdeschryver left a comment

wgd3 commented Aug 2, 2023

timdeschryver Aug 5, 2023 •

edited

Loading

timdeschryver left a comment

feat(schematics): add entity generation as part of feature schematic #3850

feat(schematics): add entity generation as part of feature schematic #3850

Conversation

wgd3 commented Apr 17, 2023

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

netlify bot commented Apr 17, 2023 • edited Loading

✅ Deploy Preview for ngrx-io ready!

wgd3 commented May 10, 2023

timdeschryver commented May 18, 2023

wgd3 commented Jun 28, 2023

brandonroberts Jun 29, 2023

Choose a reason for hiding this comment

timdeschryver left a comment

Choose a reason for hiding this comment

wgd3 commented Aug 2, 2023

timdeschryver Aug 5, 2023 • edited Loading

Choose a reason for hiding this comment

timdeschryver left a comment

Choose a reason for hiding this comment

netlify bot commented Apr 17, 2023 •

edited

Loading

timdeschryver Aug 5, 2023 •

edited

Loading