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.

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

Dependencies update: rxjs 7.3.0, TypeScript 4.4.2 #998

Merged
merged 2 commits into from
Sep 2, 2021

Conversation

peaBerberian
Copy link
Collaborator

@peaBerberian peaBerberian commented Aug 27, 2021

Ritual little dependency update.

To note that @typescript-eslint is not yet synchronized to TypeScript 4.4.2 (it was apparently released today, so we may forgive them!) and give a warning announcing it may not be compatible. So we might want to wait until they update that.

Due to some RxJS and TypeScript changes, I had to update the code:

  • Thanks to RxJS resolving Interface for ConnectableObservableLike is not exposed ReactiveX/rxjs#6529 in their last version (PR: fix: Expose Connectable, return type of connectable ReactiveX/rxjs#6531), we can now rely on the Connectable type.
    Previously we haphazardly relied on importing the ConnectableObservableLike interface from deep inside RxJS's non-API-exposed code (a solution which was not even the most elegant one at the time)

  • TypeScript got rid of the MSMediaKeys and MSMediaKeySession types for some reasons - types we use in our compatibility code for EME APIs. Here I blatantly copied their definition in previous versions in our compat code.

  • TypeScript now define catched errors as unknown (https://devblogs.microsoft.com/typescript/announcing-typescript-4-4-rc/#use-unknown-catch-variables) which I thought we already did. In reality, we may have been less zealous in tests files, where arguably ugly assertions have now been added.

  • Two calls to castToObservable now do not type-check, for reasons which seem invalid. Casting the inputed Promise<void> | Promise<SomeType> (in both cases it is the same type) as a Promise<unknown> - which I guess is always compatible - works.
    So I did just that. However relying on casting is in some cases a little unsafe, so I would have preferred not to.
    Did not think too much about that one, but the issue seems to be linked with how TypeScript now resolves generics relatively to inputted union types. Maybe we could improve on castToObservable's typing?

  • TypeScript 4.4 comes with this nice exactOptionalPropertyTypes flag (https://devblogs.microsoft.com/typescript/announcing-typescript-4-4-rc/#exact-optional-property-types) which basically treats differently an optional type (a type to which ? is appended in an interface, for example) and a possibly undefined type. The former cannot be set to undefined, the second has to be set, even if it is to set it to undefined.

    This is a nice feature that I always wanted to add because there are subtle though important differences between the two (for example, Object.assign will overwrite an undefined field with the same name than a previously-defined one). But while activating it, I noticed that a lot of different parts of the code do not pass this check, amounting to 200 or so errors.
    This is big, but not too big, so I guess we may work on it in the future, but it will necessitate its own PR.

@peaBerberian peaBerberian force-pushed the misc/dependencies_update_rxjs_7.3.0 branch from c5657f4 to 11e32f7 Compare August 27, 2021 14:33
@peaBerberian peaBerberian added the work-in-progress This Pull Request or issue is not finished yet label Sep 1, 2021
@peaBerberian peaBerberian force-pushed the misc/dependencies_update_rxjs_7.3.0 branch from 11e32f7 to 3142212 Compare September 2, 2021 15:10
@peaBerberian peaBerberian merged commit 0df82a9 into master Sep 2, 2021
@peaBerberian peaBerberian deleted the misc/dependencies_update_rxjs_7.3.0 branch December 3, 2021 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work-in-progress This Pull Request or issue is not finished yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants