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

feat: move require (local files) to source.uri #3535

Merged

Conversation

KrzysztofMoch
Copy link
Collaborator

@KrzysztofMoch KrzysztofMoch commented Feb 14, 2024

Summary

From now "local files" that are imported via require will be passed in source.uri

Motivation

This will unblock video config for local files

Changes

changed resolveAssetSourceForVideo function (JS side change only)

Test plan

  • Tested in example app

Copy link
Collaborator

@freeboub freeboub left a 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 !

@KrzysztofMoch KrzysztofMoch changed the title feat!: move require (local files) to source.uri feat: move require (local files) to source.uri Feb 16, 2024
src/utils.ts Outdated
Comment on lines 8 to 31
// 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,
};
}

Copy link
Collaborator Author

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

@freeboub
Copy link
Collaborator

@KrzysztofMoch finally successfully tested ! :)
what do you think of this addtionnal commit ? 9ab8712

@KrzysztofMoch
Copy link
Collaborator Author

what do you think of this addtionnal commit ? 9ab8712

Good idea, I have added this

@KrzysztofMoch KrzysztofMoch merged commit 41ac781 into TheWidlarzGroup:master Mar 13, 2024
3 checks passed
@KrzysztofMoch KrzysztofMoch deleted the feat/move-require-to-uri branch March 13, 2024 07:23
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

2 participants