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

fix: check directory before call delete on iOS #1066

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

camilossantos2809
Copy link
Contributor

Summary

Before executing the clear method on iOS, check if the directory exists using RCTGetStorageDirectory to avoid throwing an error.

Fixes #1020

Test Plan

To test this change I created an app from scratch and call await AsyncStorage.clear().
The component code is available on gist

before:

before.mp4

after:

after.mp4

Copy link
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

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

Thanks for submitting this fix!

@@ -869,8 +869,14 @@ - (BOOL)_passthroughDelegate

[_manifest removeAllObjects];
[RCTGetCache() removeAllObjects];
NSDictionary *error = RCTDeleteStorageDirectory();
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be simpler if we check whether error.code == NSFileNoSuchFileError in RCTDeleteStorageDirectory() and ignore it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it works and is also more clean, thanks for that.
I'm kinda new on Objective-C, I only return nil to ignore, if it was better to return another thing I could change again.

Copy link
Member

@tido64 tido64 left a comment

Choose a reason for hiding this comment

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

Thanks again. I left only a minor nitpick.

packages/default-storage/ios/RNCAsyncStorage.mm Outdated Show resolved Hide resolved
Copy link
Member

@krizzu krizzu left a comment

Choose a reason for hiding this comment

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

Thanks!

Co-authored-by: Tommy Nguyen <[email protected]>
@tido64 tido64 enabled auto-merge (squash) February 23, 2024 11:59
@tido64 tido64 merged commit 9db07c8 into react-native-async-storage:main Feb 23, 2024
10 checks passed
@AsyncStorageBot
Copy link
Collaborator

🎉 This PR is included in version 1.22.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clear should not throw if the db hasn't been created yet
4 participants