search

callstack / react-native-image-editor

A library providing an API for cropping images from the web and the local file system.
MIT License
379 stars 118 forks source link

[iOS] Don't use ImageStore #14

Closed guhungry closed 5 years ago

guhungry commented 5 years ago

Since ImageStore is deprecated should migrate out of it. And also it will have consistent behavior between ios and android.

ReactNativeRenderer-dev.js:8905 Warning: ImageStore is deprecated and will be removed in a future release. To get a base64-encoded string from a local image use either of the following third-party libraries: expo-file-system: readAsStringAsync(filepath, 'base64') react-native-fs: readFile(filepath, 'base64')

Potential Implementation

https://guides.github.com/features/mastering-markdown/ https://github.com/react-native-community/react-native-camera/blob/master/ios/RN/RNFileSystem.m

https://github.com/react-native-community/react-native-camera/blob/d6a7c625d67e05a783119b7ebcfb937c0232e55f/ios/RN/RNCamera.m#L439-L442

Trancever commented 5 years ago

Hi @guhungry Thanks for creating issue and yes, we need to migrate from using ImageStoreManager.

@cpojer Do you have any recommendations, which package should we use?

kevinwu127 commented 5 years ago

Any updates on this? ImageEditor is still saving to a location where RNFS cannot read. This should be solved before ImageStore is not supported entirely.

guhungry commented 5 years ago

@Trancever There is no need for 3rd party package. This can easily implemented in native code as in Potential Implementation.

Trancever commented 5 years ago

@guhungry @kevinwu127 Check v1.4.0 and let me know what you think!

guhungry commented 5 years ago

I'll test it tonight and report back. But i think this this is breaking change for ios?

Trancever commented 5 years ago

@guhungry I haven't thought about it as breaking change, but It makes sense, since there is no need to clear the image right now. I'll remove that v1.4 release from both npm and github tomorrow and I'll do new major release then.

guhungry commented 5 years ago

It does work as expect. Major version make sense to me.

@Trancever What do you think about using promise instead of callback?

Trancever commented 5 years ago

@guhungry I am ok with Promises, especially right now when we are releasing new major version

kevinwu127 commented 5 years ago

@Trancever Works great! Looking forward to the Promise implementation :)

benzman81 commented 5 years ago

Since you reverted the version and the ImageStore is still used, shouldn't you reopen this issue?

Trancever commented 5 years ago

@benzman81 Once #29 lands on master, I'll release new major version, but yeah, for now this should be reopened

guhungry commented 5 years ago

@benzman81 @kevinwu127 Can you help test with #29 which also includes this PR and Promise implementation?

kevinwu127 commented 5 years ago

@guhungry the PR looks good! Thank you for this :)

Trancever commented 5 years ago

I've just released version 2.0.0-alpha.0 that contains new way of storing images on iOS + Promise api. Could you guys test it and let me know if it works well for you?

guhungry commented 5 years ago

Works for me.

Screenshots Android: iOS:
benzman81 commented 5 years ago

Sorry for the delay. v2.0.0-alpha.0 works like a charm.

Trancever commented 5 years ago

@benzman81 @guhungry Awesome, I'll release a non alpha version this week.

Trancever commented 5 years ago

v2.0.0 is released.