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

RCTAsyncLocalStorage: don't reset the data when have no access to the manifest #22783

Conversation

aleh
Copy link

@aleh aleh commented Dec 24, 2018

Starting with an empty dictionary is a reasonable fallback in case the manifest cannot be parsed, however it would lead to data loss when the app temporarily has no access to its documents due to data protection being enabled and the device being locked.

Changelog:

[iOS] [Fixed] - Fix a data loss problem in RCTAsyncLocalStorage happening when data protection is enabled for the app which starts when the device is locked due to background push notification, geofence event, etc.

Test Plan:

Only checked with an app we are making. Before the fix, when data protection was turned on and the app would start due to a geofence event while the phone was locked, then it would begin with an empty state, which would be later successfully saved when the user would unlock the phone and start the app manually. After the fix, the app would silently crash in this case, which is far from ideal behavior, but at least no data loss would occur.

@aleh aleh requested a review from shergin as a code owner December 24, 2018 16:38
@facebook-github-bot
Copy link
Contributor

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 24, 2018
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@aleh
Copy link
Author

aleh commented Dec 27, 2018

Hey guys! This test fails even without my fix, just tried this on master. CC @shergin

@radeno
Copy link
Contributor

radeno commented Feb 25, 2019

@aleh @cpojer @krizzu is this critical? i think losing data is important issue.
Or should be moved to https://github.com/react-native-community/react-native-async-storage

@cpojer
Copy link
Contributor

cpojer commented Feb 25, 2019

Yeah I think this should go into the new async-storage repo. Let's close this PR here.

@aleh we moved this code out of React Native as part of the Lean Core (#23313) effort. Please reopen your PR there. I'm sure it will be possible to merge it much faster :)

@cpojer cpojer closed this Feb 25, 2019
@aleh
Copy link
Author

aleh commented Dec 27, 2019

Reopened in the async-storage repo: react-native-async-storage/async-storage#270

@aleh aleh deleted the feature/async-local-storage-data-protection-fix-2 branch January 8, 2020 14:55
facebook-github-bot pushed a commit that referenced this pull request Nov 29, 2021
Summary:
This sync includes the following changes:
- **[c1220ebdd](facebook/react@c1220ebdd )**: treat empty string as null ([#22807](facebook/react#22807)) //<salazarm>//
- **[09d9b1775](facebook/react@09d9b1775 )**: Update deprecated features in ESLint configuration files. ([#22767](facebook/react#22767)) //<Esteban>//
- **[bddbfb86d](facebook/react@bddbfb86d )**: Revert "Fix Node package.json ./ exports deprecation warning ([#22783](facebook/react#22783))" ([#22792](facebook/react#22792)) //<Sebastian Silbermann>//
- **[b831aec48](facebook/react@b831aec48 )**: chore(fast-refresh): double check wasMounted ([#22740](facebook/react#22740)) //<anc95>//
- **[8edeb787b](facebook/react@8edeb787b )**: Fix Node package.json ./ exports deprecation warning ([#22783](facebook/react#22783)) //<Rin Arakaki>//
- **[fdc1d617a](facebook/react@fdc1d617a )**: Flag for client render fallback behavior on hydration mismatch ([#22787](facebook/react#22787)) //<salazarm>//
- **[aa19d569b](facebook/react@aa19d569b )**: Add test selectors to experimental build ([#22760](facebook/react#22760)) //<Brian Vaughn>//
- **[520ffc77a](facebook/react@520ffc77a )**: Use globalThis if possible for native fetch in browser build ([#22777](facebook/react#22777)) //<Jiachi Liu>//
- **[afbc2d08f](facebook/react@afbc2d08f )**: Remove unused react-internal/invariant-args ESLint rule. ([#22778](facebook/react#22778)) //<Esteban>//
- **[ca94e2680](facebook/react@ca94e2680 )**: Remove 'packages/shared/invariant.js' ([#22779](facebook/react#22779)) //<Esteban>//
- **[83564712b](facebook/react@83564712b )**: Move SuspenseList to experimental channel ([#22765](facebook/react#22765)) //<Andrew Clark>//
- **[d4144e6e5](facebook/react@d4144e6e5 )**: fix : grammatical typo for test description ([#22764](facebook/react#22764)) //<Brijesh Prasad>//
- **[0b329511b](facebook/react@0b329511b )**: chore: fix comment typo ([#22657](facebook/react#22657)) //<Han Han>//
- **[e6f60d2ad](facebook/react@e6f60d2ad )**: fix typos ([#22715](facebook/react#22715)) //<180909>//

Changelog:
[General][Changed] - React Native sync for revisions c0c71a8...c1220eb

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D32646433

fbshipit-source-id: c534ee7a17141634700c90fc2c7b34bfbe17887a
nawbc pushed a commit to NawbExplorer/react-native that referenced this pull request Dec 7, 2021
Summary:
This sync includes the following changes:
- **[c1220ebdd](facebook/react@c1220ebdd )**: treat empty string as null ([facebook#22807](facebook/react#22807)) //<salazarm>//
- **[09d9b1775](facebook/react@09d9b1775 )**: Update deprecated features in ESLint configuration files. ([facebook#22767](facebook/react#22767)) //<Esteban>//
- **[bddbfb86d](facebook/react@bddbfb86d )**: Revert "Fix Node package.json ./ exports deprecation warning ([facebook#22783](facebook/react#22783))" ([facebook#22792](facebook/react#22792)) //<Sebastian Silbermann>//
- **[b831aec48](facebook/react@b831aec48 )**: chore(fast-refresh): double check wasMounted ([facebook#22740](facebook/react#22740)) //<anc95>//
- **[8edeb787b](facebook/react@8edeb787b )**: Fix Node package.json ./ exports deprecation warning ([facebook#22783](facebook/react#22783)) //<Rin Arakaki>//
- **[fdc1d617a](facebook/react@fdc1d617a )**: Flag for client render fallback behavior on hydration mismatch ([facebook#22787](facebook/react#22787)) //<salazarm>//
- **[aa19d569b](facebook/react@aa19d569b )**: Add test selectors to experimental build ([facebook#22760](facebook/react#22760)) //<Brian Vaughn>//
- **[520ffc77a](facebook/react@520ffc77a )**: Use globalThis if possible for native fetch in browser build ([facebook#22777](facebook/react#22777)) //<Jiachi Liu>//
- **[afbc2d08f](facebook/react@afbc2d08f )**: Remove unused react-internal/invariant-args ESLint rule. ([facebook#22778](facebook/react#22778)) //<Esteban>//
- **[ca94e2680](facebook/react@ca94e2680 )**: Remove 'packages/shared/invariant.js' ([facebook#22779](facebook/react#22779)) //<Esteban>//
- **[83564712b](facebook/react@83564712b )**: Move SuspenseList to experimental channel ([facebook#22765](facebook/react#22765)) //<Andrew Clark>//
- **[d4144e6e5](facebook/react@d4144e6e5 )**: fix : grammatical typo for test description ([facebook#22764](facebook/react#22764)) //<Brijesh Prasad>//
- **[0b329511b](facebook/react@0b329511b )**: chore: fix comment typo ([facebook#22657](facebook/react#22657)) //<Han Han>//
- **[e6f60d2ad](facebook/react@e6f60d2ad )**: fix typos ([facebook#22715](facebook/react#22715)) //<180909>//

Changelog:
[General][Changed] - React Native sync for revisions c0c71a8...c1220eb

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D32646433

fbshipit-source-id: c534ee7a17141634700c90fc2c7b34bfbe17887a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants