Skip to content

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 3, 2022
Merged

build: ease integration into projects #335

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fc0bdc
build: ease integration into projects
shaman-apprentice Dec 3, 2022
425a344
build: remove rxjs as dev dependency within library as it is handled …
shaman-apprentice Dec 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions projects/testing-library/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
"@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"
"@angular/core": ">= 14.0.0"
Copy link
Member

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

Suggested change
"@angular/core": ">= 14.0.0"
"@angular/core": ">= 14.0.0",
"rxjs": "^6.5.3 || ^7.4.0"

Copy link
Author

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?

Copy link
Member

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.

Copy link
Author

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.

},
"dependencies": {
"@testing-library/dom": "^8.0.0",
Expand Down