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

Document AsyncStorage size on iOS and Android #11656

Closed

Conversation

ericpalakovichcarr
Copy link
Contributor

@ericpalakovichcarr ericpalakovichcarr commented Dec 28, 2016

Currently, there's no documentation on the default 6MB limit on Android for AsyncStorage. This pull request adds documentation for iOS and Android size limits, as well as example code for changing the default size limit on Android.

This pull request should properly resolve this issue: #3387 (comment)

Currently, there's no documentation on the default 6MB limit on Android for `AsyncStorage`. This pull request adds documentation for iOS and Android size limits, as well as example code to changing the default size limit on Android.

This pull request should properly resolve this issue: facebook#3387 (comment)
@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @caabernathy and @davidaurelio to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Dec 28, 2016
* // Increase the maximum size of AsyncStorage
* long size = 100 * 1024L * 1024L; // 100 MB
* ReactDatabaseSupplier.getInstance(getApplicationContext()).setMaximumSize(size);
*

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

@nihgwu
Copy link
Contributor

nihgwu commented Dec 29, 2016

nice job

@mkonicek
Copy link
Contributor

mkonicek commented Dec 30, 2016

Thanks for adding docs for this!

nit: getPackages as the name says is meant to return a list of packages. How about adding the code in onCreate in the MainActivity? See https://developer.android.com/reference/android/app/Activity.html

Does that work?

@mkonicek
Copy link
Contributor

In case Activity.onCreate is too early (calling the code there crashes), overriding this method in the Application by calling super and then the extra code should work: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/ReactNativeHost.java#L65

@ericpalakovichcarr
Copy link
Contributor Author

@mkonicek Looks like both will work. Would you prefer one over the other? I'll just update the code for onCreate otherwise.

@ericpalakovichcarr
Copy link
Contributor Author

I'll just say that onCreate makes sense to me given the method is already defined in the boilerplate code generated by react-native init.

@ericpalakovichcarr
Copy link
Contributor Author

Whoops, I accidentally submitted the commit with the onCreate change. Let me know if you'd like to see the createReactInstanceManager method used instead.

@ericpalakovichcarr
Copy link
Contributor Author

Is there anything else that needs to happen here on my end? Just checking if the ticket is stuck because of me or not.

@ericpalakovichcarr
Copy link
Contributor Author

@mkonicek Is this pull request still waiting on me for something? Or do pull requests just sit in limbo for a while at times? (which is totally ok for me!)

@adrianha
Copy link
Contributor

adrianha commented Feb 11, 2017

hi @BigSassy, i already following your suggestion to update AsyncStorage's limit by modifying onCreate method but its still not working. Any idea? I'm using RN v0.40. Thank you

@ericpalakovichcarr
Copy link
Contributor Author

@adrianha Could you paste the code for your onCreate method here? I have no idea otherwise. And you're doing this in MainApplication.java right? Not MainActivity.java?

@kennethpdev
Copy link

kennethpdev commented Apr 3, 2017

@BigSassy

Here's my code on the onCreate with the latest stable version of react-native 0.42.3.

  @Override
  public void onCreate() {
    super.onCreate();
    SoLoader.init(this, false);

    // Increase the maximum size of AsyncStorage
    long size = 50L * 1024L * 1024L; // 50 MB
    ReactDatabaseSupplier.getInstance(getApplicationContext()).setMaximumSize(size);
  }

I want to increase it to have at least 50MB in size. This doesn't seem to work. I added it inside MainApplication.java. Also is there a difference by settings 50L or just 50 on the size? The original size they wrote it to 6L. Tho, changing this doesn't affect anything. AsyncStorage still not expanding for some reason.

Regards,

@hramos hramos added the Icebox label Jul 20, 2017
@hramos hramos closed this Jul 20, 2017
@hramos hramos reopened this Jul 21, 2017
@hramos hramos removed the Icebox label Jul 21, 2017
@facebook-github-bot
Copy link
Contributor

@BigSassy I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

@stale
Copy link

stale bot commented Oct 13, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. If you think this issue should definitely remain open, please let us know why. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 13, 2017
@stale stale bot closed this Oct 20, 2017
@RobIsHere
Copy link

Is this information outdated? Or why hasn't this been merged or closed normally? Having information about the limits of a system is really important for understanding its possible field of application.

Other people fall into your trap for sure! The ones who aren't that experienced and do neither anticipate nor research things like these! Caring about them costs only as much time as one click on the merge button, isn't it...?

@hramos
Copy link
Contributor

hramos commented Oct 27, 2017

There's hundreds of pull requests open and we're slowly working through them. Ideally this would have been flagged by a community member after the stale bot posted the first warning. We'll see how we can improve the process.

@hramos hramos reopened this Oct 27, 2017
@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Oct 27, 2017
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Oct 27, 2017
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Oct 28, 2017
@facebook-github-bot
Copy link
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

@hramos
Copy link
Contributor

hramos commented Dec 1, 2017

Heads up - the docs are now at https://github.com/facebook/react-native-website

You may want to make the same change over there. Sorry for the unnecessary work here, we're still in the process of landing the commits that will remove the duplicate docs from this repo. Thanks for your contribution!

@hramos hramos closed this Dec 1, 2017
@hramos hramos added Type: Docs Issues concerning the docs are tracked elsewhere: https://github.com/facebook/react-native-website and removed Import Failed labels Mar 8, 2019
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. Type: Docs Issues concerning the docs are tracked elsewhere: https://github.com/facebook/react-native-website
Projects
None yet
Development

Successfully merging this pull request may close these issues.