-
-
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 ng update #1032
Conversation
const suffix = match(firstChar, '^') || match(firstChar, '~'); | ||
|
||
// TODO: remove beta | ||
pkg.dependencies['@ngrx/store'] = `${suffix}6.0.0@beta.1`; |
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.
Why wouldn't this be 6.0.0-beta.1
since you're just modifying the package.json
? Also go ahead and change to to beta.2
since that will be the next release.
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.
Yep you're totally right!
modules/store/package.json
Outdated
@@ -19,10 +19,13 @@ | |||
"homepage": "https://github.com/ngrx/platform#readme", | |||
"peerDependencies": { | |||
"@angular/core": "NG_VERSION", | |||
"rxjs": "RXJS_VERSION" | |||
"rxjs": "RXJS_VERSION", | |||
"@angular-devkit/core": "NG_DEVKIT_VERSION", |
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 devkit peer dependencies. If you're using the Angular CLI or the Schematics CLI, we can rely on those dependencies implicitly.
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.
Also, add the NG_DEVKIT_VERSION
replacements to the modules/schematics/package.json
and bump the version to ^0.6.0
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.
Good to know, thanks!
Is this PR ready to land? Or are you going to add it to the other packages separately? |
Any idea of this is testable locally? Because I wanted to ng update a new project to see if it would update the ngrx store version. About the other packages, what would you prefer. A PR per package or should I add them all in this PR? |
Testing locally requires a local npm server. There is an article here http://www.softwarearchitekt.at/post/2018/04/17/seamlessly-updating-your-angular-libraries-with-ng-update.aspx |
Let's land this one first, then we can look at reusing some of this code across the other packages since the update path should be the same. |
OK, I think everything is fine. |
Cool. I'll merge in a bit. I have a follow-up branch ready so we can reuse it across the other packages pretty easily. |
Closes #1028