Skip to content

Settings example#12675

Closed
peterept wants to merge 1 commit into
react:masterfrom
peterept:master
Closed

Settings example#12675
peterept wants to merge 1 commit into
react:masterfrom
peterept:master

Conversation

@peterept

@peterept peterept commented Mar 3, 2017

Copy link
Copy Markdown

Hi,

I've added a settings example because the documentation doesn't have one.

I had to look through the code to understand what to pass to set.

My example uses get/set to count how many times the app was launched.

@facebook-github-bot

Copy link
Copy Markdown
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 - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact cla@fb.com if you have any questions.


render() {
return (
<Text>Launched ${this.state.launchCount} times!</Text>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Text This type is incompatible with React$Component

Settings.set({launchCount: count});

this.state = {launchCount: count};
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no-extra-semi: Unnecessary semicolon.

count++;
Settings.set({launchCount: count});

this.state = {launchCount: count};

Copy link
Copy Markdown

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.

constructor(props) {
super(props);

let count = Settings.get("launchCount") || 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

quotes: Strings must use singlequote.

super(props);

let count = Settings.get("launchCount") || 0;
count++;

Copy link
Copy Markdown

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.


render() {
return (
<Text>Launched ${this.state.launchCount} times!</Text>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

react/jsx-no-undef: 'Text' is not defined.

count++;
Settings.set({launchCount: count});

this.state = {launchCount: count};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

object literal This type is incompatible with undefined. Did you forget to declare type parameter State of property Component?


render() {
return (
<Text>Launched ${this.state.launchCount} times!</Text>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

property launchCount Property cannot be accessed on possibly undefined value undefined. Did you forget to declare type parameter State of property Component?

@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 Mar 3, 2017
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@ericvicenti

Copy link
Copy Markdown
Contributor

Is settings documented? When would somebody use settings over AsyncStorage?

@hramos

hramos commented Mar 15, 2017

Copy link
Copy Markdown
Contributor

Thanks for adding this. Do you have a test plan? This is required by the pull request template.

@greghe

greghe commented Mar 25, 2017

Copy link
Copy Markdown

@ericvicenti There are at least two reasons for using Settings over AsyncStorage. For one, Settings presents a synchronous API, so it's easy to get and set simple values without requiring async methods or promises. Second, the backing store for Settings is the same as the native app's (NSUserDefaults on iOS) which is particularly handy if you are integrating RN with an existing app and need access to the existing app's stored preferences values.

The major disadvantage to using Settings is that Settings has been implemented only for iOS (until now.) The good news is that there is a PR (#13142) which will bring Settings to the android platform.

@hramos

hramos commented May 30, 2017

Copy link
Copy Markdown
Contributor

Closing due to lack of follow up from the author.

@hramos hramos closed this May 30, 2017
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.

6 participants