-
Notifications
You must be signed in to change notification settings - Fork 94
build: ease integration into projects #335
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
build: ease integration into projects #335
Conversation
through removing rxjs from peer dependencies ref testing-library#333
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 still want to register rxjs as a peer dep.
But, we also want to support v6
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.
I think that would work for me.
I couldn't find any usage of rxjs in the library code itself. Would something not work as expected, if someone installed the library and doesn't provide rxjs? If not I would like to free the user of the obligation to install rxjs. What do you think?
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.
Oh! That's a very good point 😅 I'm so used to having it as a peer dep, but you're right we can just remove it here.
We also don't need it in the dev deps, because that's being handled at the root level.
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.
I am glad I didn't miss anything. Thanks for checking and responding so fast! 🙂 I updated the PR and removed it from dev deps as well.
...in top of monorepo ref testing-library#333
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!
@all-contributors please add @shaman-apprentice for maintenance
I've put up a pull request to add @shaman-apprentice! 🎉
@timdeschryver I don't understand how.. but this somehow got messed up during release.
12.2.1 still holds rxjs peer dependency (strictly to 7.5.6 which produces even more warnings when resolving dependencies).
❯ npm info @testing-library/angular@12.2.1 peerDependencies
{
'@angular/common': '>= 14.0.0',
'@angular/platform-browser': '>= 14.0.0',
'@angular/router': '>= 14.0.0',
'@angular/core': '>= 14.0.0',
rxjs: '7.5.6'
}
❯ npm info @testing-library/angular@12.2.0 peerDependencies
{
'@angular/common': '>= 14.0.0',
'@angular/platform-browser': '>= 14.0.0',
'@angular/router': '>= 14.0.0',
'@angular/core': '>= 14.0.0',
rxjs: '>= 7.4.0'
}
Seems to be resolved now, thanks for the follow up @dzonatan !
https://unpkg.com/@testing-library/angular@12.2.2/package.json
close #333
Thanks for being open to this change. Please let me know, if I have missed something. But I think this is the only necessary change.