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

timdeschryver merged 2 commits into testing-library:main from shaman-apprentice:build/333/rxjs-as-dev-Dependency

Dec 3, 2022

Merged

build: ease integration into projects #335

Changes from all commits

Commits

Show all changes

Select commit Hold shift + click to select a range

build: ease integration into projects

shaman-apprentice Dec 3, 2022

build: remove rxjs as dev dependency within library as it is handled ...

shaman-apprentice Dec 3, 2022

File filter

Filter by extension

Conversations

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

Show comments 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 @timdeschryver timdeschryver Dec 3, 2022 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 @shaman-apprentice shaman-apprentice Dec 3, 2022 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 @timdeschryver timdeschryver Dec 3, 2022 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. shaman-apprentice reacted with laugh emoji Copy link Author @shaman-apprentice shaman-apprentice Dec 3, 2022 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