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

[expo-updates][7/n] Remove classic updates #26080

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

wschurman
Copy link
Member

@wschurman wschurman commented Dec 21, 2023

Why

This removes the isMissingRuntimeVersion constant from expo-updates. This constant no longer makes sense since disabled is an expected mode of this library, and a warning is shown in native logs.

The code in Expo.fx.tsx would show a warning in dev, but in prod the top-level-thrown error actually wouldn't do anything except maybe prevent continued evaluation of the module code. The error wasn't shown anywhere as far as I can tell and the app still ran.

This was originally added in #11367.

Closes ENG-10957.

How

Remove it.

Test Plan

Inspect.

Checklist

Copy link

linear bot commented Dec 21, 2023

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Dec 21, 2023
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch from b89d1f0 to 079b240 Compare December 21, 2023 17:57
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-6 branch from 4e90546 to d2885e2 Compare December 22, 2023 18:44
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch from 079b240 to 3cbc661 Compare December 22, 2023 18:44
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-6 branch from d2885e2 to e937db4 Compare December 22, 2023 20:35
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch from 3cbc661 to fc1d830 Compare December 22, 2023 20:35
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-6 branch from e937db4 to 9e824cd Compare December 22, 2023 20:44
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch from fc1d830 to ca87ada Compare December 22, 2023 20:45
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-6 branch from 9e824cd to 158f617 Compare December 22, 2023 21:13
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch from ca87ada to 01c41fa Compare December 22, 2023 21:13
@wschurman wschurman marked this pull request as ready for review December 22, 2023 21:16
@wschurman wschurman requested review from EvanBacon and removed request for brentvatne and tsapeta December 22, 2023 21:16
Copy link
Member

@ide ide left a comment

Choose a reason for hiding this comment

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

LGTM. Looks like a "green flag" to remove a dependency between Expo.fx.ts and expo-updates.

For the changelog, I might suggest phrasing this as the expo package no longer does verification at run time that a runtime version is set in apps that use expo-updates, because expo-updates will disable itself when a runtime version is not defined or if the updates configuration is invalid for any other reason.

Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

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

Looks good, but please make sure to fix failures on the CI 😉

@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-6 branch from 158f617 to 68fb97f Compare January 2, 2024 22:02
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch 2 times, most recently from 9df6b9e to f475936 Compare January 2, 2024 22:04
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jan 2, 2024
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-6 branch from 68fb97f to 54c164b Compare January 3, 2024 00:38
Base automatically changed from @wschurman/remove-more-classic-updates-stuff-6 to main January 3, 2024 00:38
@wschurman wschurman force-pushed the @wschurman/remove-more-classic-updates-stuff-7 branch from f475936 to 15248c9 Compare January 3, 2024 00:39
@wschurman wschurman merged commit 8570e3f into main Jan 3, 2024
17 checks passed
@wschurman wschurman deleted the @wschurman/remove-more-classic-updates-stuff-7 branch January 3, 2024 00:39
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants