Baseflow / flutter_cache_manager

Generic cache manager for flutter
https://baseflow.com
MIT License
739 stars 426 forks source link

Await file removal to prevent unexpected results #323

Closed sidrao2006 closed 3 years ago

sidrao2006 commented 3 years ago

:arrow_heading_down: What is the current behavior?

File removal is not awaited. Due to this, calling CacheManager.removeFile appears to have flaky performance.

:new: What is the new behavior (if this is a feature change)?

File removal is awaited. The Future returned by removeFile completes when file is removed.

:boom: Does this PR introduce a breaking change?

No, but it does slow down any callers of this method which await it's result

:memo: Links to relevant issues/docs

Fixes #317

:thinking: Checklist before submitting

sidrao2006 commented 3 years ago

I think it's better to make the file.delete() awaited as the benefits of speed don't really outweigh the unexpected results.

@renefloor just to confirm, are these the changes that you recommended here?

renefloor commented 3 years ago

Yes I think that should be it, but I'll try to check better tomorrow.

sidrao2006 commented 3 years ago

@renefloor Just a reminder for reviewing this PR

sidrao2006 commented 3 years ago

@renefloor could you please review and merge this PR? Thank you

sidrao2006 commented 3 years ago

@renefloor any update on this PR? Any changes required? Thanks

sidrao2006 commented 3 years ago

@renefloor can you please publish this change by the end of the day? Thanks

renefloor commented 3 years ago

Just published as flutter_cache_manager: ^3.1.2