-
Notifications
You must be signed in to change notification settings - Fork 66
Fix file:// URLs on iOS so expo-asset works #344
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
base: main
Are you sure you want to change the base?
Conversation
Thanks ! I used it as a patch in my project, and it works 🙏 |
@Samox same ! |
Thank your PR Currently, I am temporarily using diff --git a/node_modules/rive-react-native/ios/RiveReactNativeView.swift b/node_modules/rive-react-native/ios/RiveReactNativeView.swift
index 4a218ed..81801ab 100644
--- a/node_modules/rive-react-native/ios/RiveReactNativeView.swift
+++ b/node_modules/rive-react-native/ios/RiveReactNativeView.swift
@@ -542,6 +542,9 @@ class RiveReactNativeView: RCTView, RivePlayerDelegate, RiveStateMachineDelegate
private func isValidUrl(_ url: String) -> Bool {
if let url = URL(string: url) {
+ if (url.scheme == "file") {
+ return true
+ }
return UIApplication.shared.canOpenURL(url)
} else {
return false
|
I'm trying out the new It works in dev (Expo), but in a preview build where the assets load over the I can confirm @CacaoRick's patch solved the issue. |
Do you have a fix for android as well ? Assets loaded with expo-asset (starting with |
Any input from the maintainers here? It'd be great to not need patches to get asset support! The variations of these patches seem to work well. I can simplify this PR to just the URL validation if that makes it easier to merge. |
@zhm |
I applied @CacaoRick patch and that alone worked, but this PR looks good to me and IMO should be merged along with #329. This was working before fine for ios, but this commit 3bbae70 was not accounting for assets loaded from OTA updates and the isValidURL broke it |
It seems this library will no longer be maintained. They are working on a new one. From tech lead:
https://community.rive.app/c/support/plan-to-merge-react-native-runtime-merge-request |
That's great that they're releasing a new version but a bit surprising they're abandoning this version, especially with critical bugs like this one present. If the new version uses the new architecture, there could be a lot of people stuck on this version for the foreseeable future. I have a few other patches rolling right now for other critical bugs. Looks like I'm stuck maintaining my own fork for the time being. |
Can't for the life of me make this work, is it supposed to just be something like this? On my component <Rive
url="file://assets/rive/logo.riv"
style={{ width: 400, height: 400 }}
/> On app.json under [
"expo-asset",
{
"assets": ["./assets/rive/logo.riv"]
}
] I have checked that the asset is loaded correctly by expo assets, and the rive component works perfectly with https:// urls, so no idea what the issue is. |
I advise you to give up early; they probably have no intention of maintaining this React Native Runtime. |
This PR fixes
file://
URLs on iOS so they can be loaded using standardexpo-asset
patterns.I'm not completely sure, but I think
file://
URLs did work at some point on iOS, at least according to comments on #241.I think some changes in #336 might've made them stop working. This PR fixes the issue with the
file://
URLs not passing theisValidUrl
test, and also reads them directly from disk instead of passing them through the HTTP stack.I tried to keep the changes fairly minimal hoping to make it easier to review. I tested it locally with both local and remote URLs and it works for me in production builds.