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][android][ios] allow reloading in dev mode in store clients #10310

Merged
merged 3 commits into from
Sep 24, 2020

Conversation

esamelson
Copy link
Contributor

Why

less urgent fix than #10308 and #10309, review can wait if needed

There was a small regression with the Updates JS module API in SDK 39, namely that it is no longer possible to use reloadAsync in development mode in the store clients. This PR fixes that behavior for the store clients.

How

Add a separate canRelaunch check that the module will call, and always return true in the clients, rather than checking to see if expo-updates is being used to load the app. This works because we can always reload in the store clients even if expo-updates is not being used (e.g. in development mode).

Test Plan

Test Updates.reloadAsync() in development sandbox in both UNVERSIONED and SDK 39.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2020

Fails
🚫

📋 Missing Changelog

🛠 Add missing entries to:

🛠 Suggested fixes:

📋 Missing changelog

Apply suggested changes:

diff --git a/packages/expo-updates/CHANGELOG.md b/packages/expo-updates/CHANGELOG.md
index 70a21894..17f6010f 100644
--- a/packages/expo-updates/CHANGELOG.md
+++ b/packages/expo-updates/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### 🐛 Bug fixes
 
+- allow reloading in dev mode in store clients. ([#10310](https://github.com/expo/expo/pull/10310) by [@esamelson](https://github.com/esamelson))
+
 ## 0.3.4 — 2020-09-22
 
 ### 🐛 Bug fixes

Generated by 🚫 dangerJS against 985ac80

@github-actions
Copy link
Contributor

Native Component List for this branch is ready

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.

We'll need to cherry-pick this back and remember to rebuild the SDK 39 AAR.

packages/expo-updates/src/Updates.ts Outdated Show resolved Hide resolved
esamelson and others added 2 commits September 23, 2020 17:18
Co-authored-by: James Ide <ide@users.noreply.github.com>
@esamelson
Copy link
Contributor Author

no need to rebuild the AAR as versioned expoview code is built from source, and this PR already backports the necessary changes. will land now and cherry-pick to sdk-39

@esamelson esamelson merged commit 7e43e88 into master Sep 24, 2020
@esamelson esamelson deleted the @eric/updates-reload-dev branch September 24, 2020 00:21
@brentvatne
Copy link
Member

#10464 (comment)

@Nantris
Copy link
Contributor

Nantris commented Oct 22, 2020

@brentvatne we're still seeing Updates.reloadAsync() failing in development, saying it's not supported in development.

We're running SDK 39 and using 39.0.3.tar.gz. Is there something I'm overlooking?

@brentvatne
Copy link
Member

@slapbox - probably you don't have expo-updates@0.3.5 in your project

@Nantris
Copy link
Contributor

Nantris commented Oct 22, 2020

@brentvatne you're right! Thank you, much appreciated!

Quick follow-up, is there a place we can consult for dependency versions?

We reference the bundledNativeModules.json for dependency versions by SDK version due to an issue with yarn/expo/monorepos where we can't use expo install [some package] - but those can become out of date once the SDK starts getting patches I see.

@brentvatne
Copy link
Member

brentvatne commented Oct 22, 2020

@slapbox - it's actually not out of date because it specifies a version range. we don't update it every time we release a patch version bump because we'd have too many releases of the expo package if we did that. if you ran yarn upgrade then ~0.3.3 would end up being updated to ~0.3.5

@Nantris
Copy link
Contributor

Nantris commented Oct 23, 2020

Ah good call. We usually just run yarn, not yarn upgrade. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants