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

[core] add reloadAppAsync support #28400

Merged
merged 8 commits into from May 2, 2024
Merged

[core] add reloadAppAsync support #28400

merged 8 commits into from May 2, 2024

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Apr 23, 2024

Why

close ENG-11482

How

introduce reloadAppAsync() from the expo package

some reflection code could be removed after facebook/react-native#44223 landed

Test Plan

add NCL ExpoCoreModule to test the reload function

Checklist

Copy link

linear bot commented Apr 23, 2024

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Apr 23, 2024
@expo-bot
Copy link
Collaborator

expo-bot commented Apr 23, 2024

The Pull Request introduced fingerprint changes against the base commit: 260acec

Fingerprint diff
[
  {
    "type": "dir",
    "filePath": "../../packages/expo-modules-core/android",
    "reasons": [
      "expoAutolinkingAndroid"
    ],
    "hash": "c01acc6136594938e1d732c88168a962fed8631e"
  },
  {
    "type": "dir",
    "filePath": "../../packages/expo/android",
    "reasons": [
      "expoAutolinkingAndroid"
    ],
    "hash": "69f961fbf9ccaff3305390a8b0e75940e97abede"
  }
]

Generated by PR labeler 🤖

@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Apr 24, 2024
@Kudo Kudo changed the title [core] add reloadAsync support [core] add reloadAppAsync support Apr 25, 2024
@Kudo Kudo marked this pull request as ready for review April 25, 2024 09: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.

If the app has installed a new update, will this new reloadAppAsync() method apply it? I anticipate two questions people will have are: why are there two reload methods and which one should I use?

@Kudo
Copy link
Contributor Author

Kudo commented Apr 25, 2024

If the app has installed a new update, will this new reloadAppAsync() method apply it? I anticipate two questions people will have are: why are there two reload methods and which one should I use?

no. reloadAppAsync() is just to reload the app. Updates.reloadAsync() will further change the loading bundle if there're newer updates. the use case is slightly different. since it's Updates.reloadAsync(), it has updates concept involved. does the name clear enough?

@ide
Copy link
Member

ide commented Apr 25, 2024

I think the names are clear but also think it would be helpful in the docs for each function to explain how it is different than the other function.

@expo-bot expo-bot removed the bot: passed checks ExpoBot has nothing to complain about label May 2, 2024
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label May 2, 2024
@Kudo
Copy link
Contributor Author

Kudo commented May 2, 2024

I think the names are clear but also think it would be helpful in the docs for each function to explain how it is different than the other function.

tried to update here. please help to review if the description is clear

  • * Unlike the [`Updates.reloadAsync()`](https://docs.expo.dev/versions/latest/sdk/updates/#updatesreloadasync),
    * this function does not change the loading bundle. It only reloads with the same JavaScript bundle.
  • * Unlike `Expo.reloadAppAsync()` serving from the `expo` package,
    * this function not only reloads the app but also changes the loading JavaScript bundle to the most recently downloaded one.

packages/expo-updates/src/Updates.ts Outdated Show resolved Hide resolved
packages/expo-modules-core/src/reload.ts Outdated Show resolved Hide resolved
Kudo and others added 2 commits May 2, 2024 13:41
Co-authored-by: James Ide <ide@users.noreply.github.com>
@expo-bot
Copy link
Collaborator

expo-bot commented May 2, 2024

Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines.

I've found some issues in your pull request that should be addressed (click on them for more details) 👇

⚠️ Suggestion: Missing changelog entries


Your changes should be noted in the changelog. Read Updating Changelogs guide and consider adding an appropriate entry to the following changelogs:


Generated by ExpoBot 🤖 against 651472c

@wschurman wschurman removed their request for review May 2, 2024 13:18
@Kudo
Copy link
Contributor Author

Kudo commented May 2, 2024

i was waiting for 0.74.1 for the changes from core and not using the reflection on android. since we may like to submit expo-go beforehand, i'm going to merge this first. later when we upgrade 0.74.1, i will remove the reflection.

@Kudo Kudo merged commit e906886 into main May 2, 2024
22 of 24 checks passed
@Kudo Kudo deleted the @kudo/core-reloadasync branch May 2, 2024 20:09
@brentvatne brentvatne added the published Changes from the PR have been published to npm label May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: suggestions ExpoBot has some suggestions 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

6 participants