Skip to content
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

@uppy/angular: fix Angular version requirement in peerDeps #5067

Merged
merged 1 commit into from
Apr 10, 2024
Merged

@uppy/angular: fix Angular version requirement in peerDeps #5067

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions packages/@uppy/angular/projects/uppy/angular/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
"tslib": "^2.0.0"
},
"peerDependencies": {
"@angular/common": "^16.2.0",
"@angular/core": "^16.2.0",
"@angular/common": "^17.3.0",
"@angular/core": "^17.3.0",
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know it doesn't work on both? I was wondering if we postponed this for so long while both might have been possible all along.

#4893 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't test it – and having two Angular examples to test this out seems like more work than it's worth.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also don't test 3 React versions but we explicitly support three version ranges. It's not about testing but about expertise about the framework to know how you ship a library for it. What would've been more sensible to me is testing that locally and if it works, we could've just added || ^17.0.0 instead of letting users wait five months before we ship a breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if someone was proposing this change, with some commitment that they would fix the potential bugs, it would be gladly accepted. Nobody is doing that, and I'm afraid we lack someone in the Uppy team who would be following the Angular project and could make some informed decision regarding whether it would make sense to document support for more versions than the one we test.

"@uppy/core": "workspace:^",
"@uppy/dashboard": "workspace:^",
"@uppy/drag-drop": "workspace:^",
Expand Down
Loading