feat: Require rxjs 7.5, update all deps #47
Conversation
packages/core/package.json
Outdated
| @@ -18,10 +18,10 @@ | |||
| "tslib": "^2.4.0" | |||
| }, | |||
| "peerDependencies": { | |||
| "react": ">=16.9.0" | |||
| "react": ">=18.0.0" | |||
There was a problem hiding this comment.
Why do we need this to be >= 18?
The spicy hooks are used also on few other projects, so if there's no strong reason to upgrade the minimum peer version, I wouldn't do it. Can't we just ensure compatibility with at least both 17 and 18?
18 seems to have some nasty side-effects, so the adoption might be slow.
There was a problem hiding this comment.
I guess we don't. I will revert it back for now.
packages/observables/package.json
Outdated
| "react": ">=18.0.0", | ||
| "rxjs": ">=7.5.0" |
There was a problem hiding this comment.
Same as above, do we need to bump the majors here?
There was a problem hiding this comment.
For react, I will revert it just as above. There is relatively minimal reliance on it besides the hooks the only important v18 change, the children prop, is easy to make backwards compatible.
On the other hand, rxjs is used quite heavily. Especially in package about observables. I need to look at v6 vs v7 changes. But in general it just made sense to me to require matching version. But if we know it will not introduce any issues if people still use it with v6 we could keep it as it was.
There was a problem hiding this comment.
Just looking at the list at https://rxjs.dev/deprecations/breaking-changes I would rather keep the requirement for rxjs v7.
| complete: () => { | ||
| pending$.toPromise().finally(() => subscriber.complete()).catch(noop) | ||
| } |
There was a problem hiding this comment.
Out of curiosity, what happens if you return the promise?
There was a problem hiding this comment.
It does not match the types and @typescript-eslint/no-misused-promises complains. Same with all the other changes. Alternatively I could disable the rule and revert all of it how it was before. Or just selectively the affected ones. To make sure all the functionality is the same.
There was a problem hiding this comment.
Thanks for the explanation. No need to take any action. Please let's just revert the minimum version for React and land this
There was a problem hiding this comment.
Peer dependencies already reverted to >=16.9.0. Though devDependencies use types for v18 and root requires v18 as well.
yuqiora
force-pushed
the
lerna-lite-and-preset
branch
from
6c0feed to
27b9e0c
Compare
yuqiora
force-pushed
the
update-deps-20221027
branch
from
89e1a48 to
b2cfa91
Compare
yuqiora
force-pushed
the
update-deps-20221027
branch
from
b2cfa91 to
ef7bb8b
Compare
yuqiora
changed the title
* feat!: Require react 18 and rxjs 7.5, update all deps * feat: Update various deps, lower react requirement
Updated basically all the dependencies so that we are on the latest version. Except maybe
chalkwhich remains on v4 because v5 went full ESM. Just a few extra adjustments were necessary. Given that most of it are dev deps it should hopefully be alright.When all of this is merged I might enable dependabot here. To keep it more automated.