-
-
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(store): add support of standalone API for ng-add store #3874
feat(store): add support of standalone API for ng-add store #3874
Conversation
allow to set up store for the standalone application when ng-add schematic is executed to improve DX Closes #3536
✅ Deploy Preview for ngrx-io ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
I hope I am on time :) I was proud to contribute to NgRx and looking forward to your feedback. I hope I didn't forget anything important. |
@@ -52,3 +56,20 @@ export function isLib( | |||
|
|||
return project.projectType === 'library'; | |||
} | |||
|
|||
export function getProjectMainFile( |
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.
We keep our schematics-core
synced across packages for consistency. This should be moved to
https://github.com/ngrx/platform/blob/master/modules/schematics-core/utility/project.ts
And then run
yarn copy:schematics
To copy it to schematics-core
folder in the other packages.
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.
Hm... I just tried to do the steps you described but it seems like the yarn copy:schematics
does nothing in my case. I expect the code under module/schematics-core
will be propagated to store/schematics-core/...
, however, I see no changes after running this command. Am I missing something?
P.S. The output of yarn copy:schematics
shows that the task was successfully executed.
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.
Hmm maybe it's an operating system path issue in our script. To not hold this up, if everything is good, we'll merge it and I'll follow up with the schematics-core move.
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.
Alright, so from my side no actions are needed anymore? :)
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.
Right :)
@@ -50,6 +50,13 @@ export async function createWorkspace( | |||
appTree | |||
); | |||
|
|||
appTree = await schematicRunner.runExternalSchematic( |
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.
Same as other comment about schematics-core
Great work @DMezhenskyi! Only change is moving the |
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.
Approving for functional changes
Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
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.
🚀
Thanks @DMezhenskyi! 🥳 |
allow to set up the store for the standalone application when ng-add schematic is executed to improve DX
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The execution of
ng add @ngrx/store
fails for the standalone angular applicationCloses #3536
What is the new behavior?
The execution of
ng add @ngrx/store
will configure the store for a standalone application if the--standalone
flag is providedDoes this PR introduce a breaking change?
Other information