-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat: move require (local files) to source.uri
#3535
feat: move require (local files) to source.uri
#3535
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No change on native side ?
Can you also update the sample to be able to test easily ?
I would also expect a documentation update !
source.uri
source.uri
src/utils.ts
Outdated
// This is deprecated, but we need to support it for backward compatibility | ||
if (typeof source === 'number') { | ||
return { | ||
uri: Image.resolveAssetSource(source).uri, | ||
}; | ||
} | ||
|
||
if (typeof source.uri === 'number') { | ||
return { | ||
...source, | ||
uri: Image.resolveAssetSource(source.uri).uri, | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freeboub In stand of making breaking change I will leave old option and add reference in docs to use new way. With this we won't have to make breaking change
No native side change ?
Yes as you can see, we earlier converted require
into { uri: "some uri to local file" }
so basically I've just added one if statement
@KrzysztofMoch finally successfully tested ! :) |
Good idea, I have added this |
Summary
From now "local files" that are imported via
require
will be passed insource.uri
Motivation
This will unblock video config for local files
Changes
changed
resolveAssetSourceForVideo
function (JS side change only)Test plan