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

Upgrade SDWebImage 5.0, using SDAnimatedImageView instead of FLAnimatedImageView for better animation support #454

Closed

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Apr 23, 2019

Also, fix the bug of cacheOnly behavior on iOS

Changes

  1. The dependency of iOS(SDWebImage) upgrade to 5.0
  2. The FLAnimatedImage implementation is dropped, since SDWebImage 5.0 provide built-in solution for animation images, which is better than FLAnimatedImage (Tied to GIF only, no APNG/WebP support), learn more here: Wiki - Animated Image
  3. Fix the bug of cacheOnly attribute, it should not request any network. However, previously, it does a wrong behavior, which request the network, but only store to memory cache (This option now renamed to SDWebImageContextStoreCacheType[@(SDImageCacheTypeMemory)] context option)

Related

#447

…edImageView for better animation support.

Fix the bug of `cacheOnly` behavior
@codecov
Copy link

codecov bot commented Apr 23, 2019

Codecov Report

Merging #454 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #454   +/-   ##
=======================================
  Coverage   94.73%   94.73%           
=======================================
  Files           1        1           
  Lines          19       19           
=======================================
  Hits           18       18           
  Misses          1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a3438c...14b19cc. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 23, 2019

Codecov Report

Merging #454 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #454   +/-   ##
=======================================
  Coverage   94.73%   94.73%           
=======================================
  Files           1        1           
  Lines          19       19           
=======================================
  Hits           18       18           
  Misses          1        1
Impacted Files Coverage Δ
src/index.js 94.73% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14ea01c...cc1a03d. Read the comment docs.

@dreampiggy
Copy link
Contributor Author

@DylanVann Hi there.
I'm a collaborator of SDWebImage, Could you please have a review for this ?

SDWebImage 5.0 provide many many feature from 4.0, for better customizable, better animation support (A brand new solution for all animated image), better decoder plugins for massive image format. I think it's better to migrate this upstream dependency.

See more : https://github.com/SDWebImage/SDWebImage/releases/tag/5.0.0

@@ -20,8 +20,5 @@ Pod::Spec.new do |s|
s.exclude_files = "ios/Vendor/**/*.{h,m}"

s.dependency 'React'
s.dependency 'SDWebImage/Core'
s.dependency 'SDWebImage/GIF'
s.dependency 'SDWebImage/WebP'
Copy link
Contributor Author

@dreampiggy dreampiggy Apr 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 5.0, all external image format (Not supported by Apple's built-in framework), is a coder plugin.

The user, can choose to use any format, don't need a CocoaPods subspec for this. Which make it more modular and under control. See more on : https://github.com/SDWebImage/SDWebImage/wiki/Coder-Plugin-List

For example, if user want to use WebP format, they can do this:

pod 'react-native-fast-image', :path => '../node_modules/react-native-fast-image'
pod 'SDWebImageWebPCoder'

Then register the coder plugin, in AppDelegate.m

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
// ...
// Register WebP format support
[SDImageCodersManager.sharedManager addCoder:SDImageWebPCoder.sharedCoder];
// ...
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would mean that by default this module would no longer support as many formats though right?

Copy link
Contributor Author

@dreampiggy dreampiggy May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would mean that by default this module would no longer support as many formats though right?

Whether SDWebImage supports some format, or not. It's now, totally controlled by the users (the Pod integrator). If users want, they can supports all the format, listed in : Coder Plugin List.

The main repo (SDWebImage pod itself), only supports the built-in format by Apple's SDK. Typically the JPEG/PNG/APNG/TIFF/GIF/HEIF, etc.

As I see, react-native-fast-image SDK, didn't rely on any dependency on what original image format is. Isn't it ? We just care about the animation, and their meta info (loop count/ frame count), but not what detailed the original compressed data is. Both APNG/GIF/WebP/BPG(more) supports animation, they can be abstracted as the same.

@dreampiggy
Copy link
Contributor Author

Hi. Seems this PR is update to the master branch. And I've answered that question. Any new update about this ?
@DylanVann Do you have time recently ? If there are anything I can do to make this merged, please feel free to reply.

@DylanVann
Copy link
Owner

I think this is good assuming it supports formats we used to support without manual steps. Just need time to test it out, maybe tonight. Thank you!

DylanVann pushed a commit that referenced this pull request May 8, 2019
This uses SDAnimatedImageView instead of FLAnimatedImageView for better animation support.

BREAKING CHANGE: Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior

fix #447
@DylanVann
Copy link
Owner

Had to make some modifications but it's in. I wish there was a better process for this kind of thing using semantic release...

Thank you @dreampiggy !

@DylanVann DylanVann closed this May 8, 2019
DylanVann pushed a commit that referenced this pull request May 8, 2019
# [6.0.0](v5.4.2...v6.0.0) (2019-05-08)

### Features

* Upgrade to SDWebImage 5.0. ([#454](#454)) ([8a216e2](8a216e2)), closes [#447](#447)

### BREAKING CHANGES

* Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior
luffy1701 pushed a commit to luffy1701/react-native-fast-image-v2 that referenced this pull request May 9, 2019
This uses SDAnimatedImageView instead of FLAnimatedImageView for better animation support.

BREAKING CHANGE: Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior

fix DylanVann#447
luffy1701 pushed a commit to luffy1701/react-native-fast-image-v2 that referenced this pull request May 9, 2019
# [6.0.0](DylanVann/react-native-fast-image@v5.4.2...v6.0.0) (2019-05-08)

### Features

* Upgrade to SDWebImage 5.0. ([DylanVann#454](DylanVann#454)) ([8a216e2](DylanVann@8a216e2)), closes [DylanVann#447](DylanVann#447)

### BREAKING CHANGES

* Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior
@guhungry guhungry mentioned this pull request May 24, 2019
@dreampiggy dreampiggy deleted the upgrade_sdwebimage_5.x branch June 2, 2019 03:55
charleswong28 pushed a commit to EONIQ/react-native-fast-image that referenced this pull request Jul 1, 2019
This uses SDAnimatedImageView instead of FLAnimatedImageView for better animation support.

BREAKING CHANGE: Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior

fix DylanVann#447
charleswong28 pushed a commit to EONIQ/react-native-fast-image that referenced this pull request Jul 1, 2019
# [6.0.0](DylanVann/react-native-fast-image@v5.4.2...v6.0.0) (2019-05-08)

### Features

* Upgrade to SDWebImage 5.0. ([DylanVann#454](DylanVann#454)) ([8a216e2](DylanVann@8a216e2)), closes [DylanVann#447](DylanVann#447)

### BREAKING CHANGES

* Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior
github-actions bot pushed a commit to perrystreetsoftware/react-native-fast-image that referenced this pull request Aug 3, 2021
# [6.0.0](v5.0.11...v6.0.0) (2021-08-03)

### Bug Fixes

* add react@17 as peer dependency ([DylanVann#790](https://github.com/perrystreetsoftware/react-native-fast-image/issues/790)) ([27bd586](27bd586))
* xcode 12 compatibility ([DylanVann#732](https://github.com/perrystreetsoftware/react-native-fast-image/issues/732)) ([23c3955](23c3955))
* **android:** make center ResizeMode work correctly ([d648ef8](d648ef8))
* **android:** remove explicit use of UI thread ([DylanVann#698](https://github.com/perrystreetsoftware/react-native-fast-image/issues/698)) ([5d2894e](5d2894e))
* accessibilityIgnoresInvertColors prop not recognised when using TypeScript ([DylanVann#666](https://github.com/perrystreetsoftware/react-native-fast-image/issues/666)) ([22f89e4](22f89e4)), closes [/github.com/DylanVann/react-native-fast-image/blob/master/src/index.tsx#L150-L160](https://github.com//github.com/DylanVann/react-native-fast-image/blob/master/src/index.tsx/issues/L150-L160)
* Add git tag to CocoaPods source property ([DylanVann#601](https://github.com/perrystreetsoftware/react-native-fast-image/issues/601)) ([2d706ad](2d706ad))
* Add tintColor type definition. ([4adf42f](4adf42f))
* Bump Glide version number to v4.11.0. ([DylanVann#649](https://github.com/perrystreetsoftware/react-native-fast-image/issues/649)) ([c4e4306](c4e4306)), closes [DylanVann#536](https://github.com/perrystreetsoftware/react-native-fast-image/issues/536)
* Fix dependency versions not specified in podfile. ([89f3379](89f3379)), closes [DylanVann#456](https://github.com/perrystreetsoftware/react-native-fast-image/issues/456)
* Fix fallback prop not working. ([DylanVann#420](https://github.com/perrystreetsoftware/react-native-fast-image/issues/420)) ([487d410](487d410))
* Fix IllegalArgumentException crash (Android). ([DylanVann#511](https://github.com/perrystreetsoftware/react-native-fast-image/issues/511)) ([b6c4677](b6c4677))
* Fix incorrect syntax. ([11f6047](11f6047))
* Fix local resource cache issue on Android. ([DylanVann#472](https://github.com/perrystreetsoftware/react-native-fast-image/issues/472)) ([5f65383](5f65383)), closes [DylanVann#402](https://github.com/perrystreetsoftware/react-native-fast-image/issues/402)
* Fix memory leak on iOS. ([DylanVann#433](https://github.com/perrystreetsoftware/react-native-fast-image/issues/433)) ([70be744](70be744))
* Fix peer dependency and remove prop-types. ([44a4c8b](44a4c8b))
* Fix setting props order issue for iOS. ([DylanVann#303](https://github.com/perrystreetsoftware/react-native-fast-image/issues/303)) ([5597ed0](5597ed0)), closes [DylanVann#304](https://github.com/perrystreetsoftware/react-native-fast-image/issues/304)
* Fix wildcard peer dependencies. ([7149420](7149420)), closes [DylanVann#440](https://github.com/perrystreetsoftware/react-native-fast-image/issues/440)
* Fixes cacheControl types. ([DylanVann#382](https://github.com/perrystreetsoftware/react-native-fast-image/issues/382)) ([e13db7d](e13db7d)), closes [DylanVann#325](https://github.com/perrystreetsoftware/react-native-fast-image/issues/325)
* Fixes podspec syntax. ([b627646](b627646))
* Fixes WebP rendering on iOS 12. ([DylanVann#412](https://github.com/perrystreetsoftware/react-native-fast-image/issues/412)) ([97630c8](97630c8)), closes [DylanVann#298](https://github.com/perrystreetsoftware/react-native-fast-image/issues/298) [DylanVann#385](https://github.com/perrystreetsoftware/react-native-fast-image/issues/385)
* Loading images by reverting "bug: Use device scale when loading images.". ([0326c3e](0326c3e)), closes [DylanVann#509](https://github.com/perrystreetsoftware/react-native-fast-image/issues/509)
* peer dependency warning ([DylanVann#653](https://github.com/perrystreetsoftware/react-native-fast-image/issues/653)) ([cd81b1b](cd81b1b))
* remove cache property if using fallback ([ba0f238](ba0f238))
* Replace 'Component' with 'ComponentType' ([DylanVann#647](https://github.com/perrystreetsoftware/react-native-fast-image/issues/647)) ([6abb273](6abb273))
* update SDWebImage and SDWebImageWebPCoder ([DylanVann#689](https://github.com/perrystreetsoftware/react-native-fast-image/issues/689)) ([9646456](9646456))
* Updates SDWebImageWebPCoder. ([DylanVann#628](https://github.com/perrystreetsoftware/react-native-fast-image/issues/628)) ([325d77f](325d77f))
* Upgrade vendored SDWebImage to v5.0.5. ([5016172](5016172)), closes [DylanVann#489](https://github.com/perrystreetsoftware/react-native-fast-image/issues/489)
* wrong cache type ([DylanVann#688](https://github.com/perrystreetsoftware/react-native-fast-image/issues/688)) [skip ci] ([94e2256](94e2256))

### Features

* **ios:** allow for for per-image-request-headers ([DylanVann#691](https://github.com/perrystreetsoftware/react-native-fast-image/issues/691)) ([4a7cd64](4a7cd64))
* Add cookie support for iOS. ([DylanVann#284](https://github.com/perrystreetsoftware/react-native-fast-image/issues/284)) ([ae47bff](ae47bff))
* Add tint color support. ([03c50f0](03c50f0)), closes [DylanVann#124](https://github.com/perrystreetsoftware/react-native-fast-image/issues/124)
* Add tvOS target. ([DylanVann#486](https://github.com/perrystreetsoftware/react-native-fast-image/issues/486)) ([6805972](6805972))
* converts to TypeScript ([DylanVann#642](https://github.com/perrystreetsoftware/react-native-fast-image/issues/642)) ([ac11706](ac11706))
* export ResizeMode and Priority types ([DylanVann#678](https://github.com/perrystreetsoftware/react-native-fast-image/issues/678)) ([e33664f](e33664f))
* Upgrade example apps. ([DylanVann#453](https://github.com/perrystreetsoftware/react-native-fast-image/issues/453)) ([25f8f0d](25f8f0d))
* Upgrade to React Native 0.60.0 / CocoaPods / Android X. ([DylanVann#513](https://github.com/perrystreetsoftware/react-native-fast-image/issues/513)) ([5489f9e](5489f9e))
* Upgrade to SDWebImage 5.0. ([DylanVann#454](https://github.com/perrystreetsoftware/react-native-fast-image/issues/454)) ([8a216e2](8a216e2)), closes [DylanVann#447](https://github.com/perrystreetsoftware/react-native-fast-image/issues/447)
* Use forwardRef to allow access to ref.measure and others. ([DylanVann#419](https://github.com/perrystreetsoftware/react-native-fast-image/issues/419)) ([2b4fba3](2b4fba3)), closes [DylanVann#69](https://github.com/perrystreetsoftware/react-native-fast-image/issues/69)

### Performance Improvements

* Use React.memo for FastImage. ([DylanVann#449](https://github.com/perrystreetsoftware/react-native-fast-image/issues/449)) ([5c2b4af](5c2b4af))

### Reverts

* Remove functionality for notifying other images on load. ([DylanVann#452](https://github.com/perrystreetsoftware/react-native-fast-image/issues/452)) ([292223d](292223d))

### BREAKING CHANGES

* This changes how network requests are handled on iOS. Make sure they still work for you.
* You should upgrade React Native. See https://facebook.github.io/react-native/blog/2019/07/03/version-60
* Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior
alicayan008 pushed a commit to alicayan008/ReactNative-fast-image that referenced this pull request Jul 4, 2023
# [6.0.0](DylanVann/react-native-fast-image@v5.4.2...v6.0.0) (2019-05-08)

### Features

* Upgrade to SDWebImage 5.0. ([#454](DylanVann/react-native-fast-image#454)) ([8a216e2](DylanVann/react-native-fast-image@8a216e2)), closes [#447](DylanVann/react-native-fast-image#447)

### BREAKING CHANGES

* Upgrade SDWebImage, may affect some projects and CocoaPods users.

Fix the bug of `cacheOnly` behavior
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.

2 participants