[expo-updates] Make error messages consistent across platforms for dev client and disabled controllers #26988
Conversation
…v client and disabled controllers
| @@ -318,19 +318,19 @@ public final class DevLauncherAppController: NSObject, InternalAppControllerInte | |||
| } | |||
|
|
|||
| public func checkForUpdate(success successBlockArg: @escaping (CheckForUpdateResult) -> Void, error errorBlockArg: @escaping (ExpoModulesCore.Exception) -> Void) { | |||
| errorBlockArg(NotAvailableInDevClientException()) | |||
| errorBlockArg(NotAvailableInDevClientException("check for update")) | |||
There was a problem hiding this comment.
should we use the js method names here, so people know exactly what to search their code for? so, checkForUpdateAsync in this case
There was a problem hiding this comment.
I agree with this, we should use the public API names
There was a problem hiding this comment.
Potentially! I'm curious how we'd phrase it. Something like: "checkForUpdateAsync cannot be called from a development client. A non-development build should be used to test this functionality." ?
There was a problem hiding this comment.
It's not that it can't be called, it's more that while it can be called it will always throw. So maybe "checkForUpdateAsync is not available in development client builds. A non-development build should be used to test this functionality."
There was a problem hiding this comment.
maybe:
Updates.checkForUpdateAsync() cannot be called from a development build. A release build should be used to test this functionality.
There was a problem hiding this comment.
The "release build" is technically incorrect as well since a debug build can also be used if the correct EX_UPDATE_NATIVE_DEBUG flags are set as well. The set of builds that need to be used are "another type of build build except one that uses dev client".
There was a problem hiding this comment.
maybe we can remove the second sentence then?
also a slight tweak to wording maybe:
Updates.checkForUpdateAsync() was called, but it is a no-op in development builds.
There was a problem hiding this comment.
It isn't a no-op, it will throw an exception...
There was a problem hiding this comment.
I like the idea of making it a single sentence that implies that the inverse of a development build is where it is supported. I went with:
Updates.checkForUpdateAsync() is not supported in development builds.
expo-bot
added
bot: passed checks
wschurman
deleted the
@wschurman/make-expo-updates-exceptions-consistent
branch
Why
These messages should be more descriptive on iOS. They were already better on android since android exceptions are easier to use, but we can do some string interpolation on iOS to achieve the same set of error messages without needing to create a separate exception subclass for each error.
How
Update messages.
Test Plan
Inspect.
Checklist
npx expo prebuild& EAS Build (eg: updated a module plugin).