-
Notifications
You must be signed in to change notification settings - Fork 61
STITCH-1732 Create React Native SDK #160
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
Conversation
edaniels
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just some random comments.
| expect(stitchUserProfileImpl.pictureUrl).toEqual(PICTURE_URL); | ||
| expect(stitchUserProfileImpl.minAge).toEqual(Number.parseInt(MIN_AGE)); | ||
| expect(stitchUserProfileImpl.maxAge).toEqual(Number.parseInt(MAX_AGE)); | ||
| expect(stitchUserProfileImpl.minAge).toEqual(Number.parseInt(MIN_AGE, 10)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new linter warning
| * Initializes the default StitchAppClient associated with the application. | ||
| * | ||
| * @param clientAppId The desired clientAppId for the client. | ||
| * @param config Additional configuration options (optional). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc return type
| * @param clientAppId The desired clientAppId for the client. | ||
| * @param config Additional configuration options (optional). | ||
| */ | ||
| public static initializeAppClient( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doc return type
| StitchUserIdentity, | ||
| Storage, | ||
| MemoryStorage, | ||
| RNAsyncStorage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you actually want to expose this to users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, good catch. I was using it when debugging earlier but I can get rid of it here.
| { | ||
| "name": "mongodb-stitch-react-native-coretest", | ||
| "version": "4.0.9", | ||
| "description": "web core test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update descs in these new files
packages/react-native/core/README.md
Outdated
|
|
||
| This package contains the React Native Core. | ||
|
|
||
| **For more information on using this on a server, see the [mongodb-stitch-react-native-sdk](https://www.npmjs.com/package/mongodb-stitch-react-native-sdk) package.** No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on react native
This is ready to review, and almost ready to publish, I just need to update the READMEs.
Main things to note:
mongodb-stitch-react-native-coreareRNAsyncStorage.tsandStitch.ts(since we now have async client initialization),react-nativeand several other packages are in a particular project'snode_modulesso I've added some packages tonohoistinlerna.json. This only affects the react native projects.react-nativeas a dependency, it is only a peer dependency ofmongodb-stitch-react-native-core. React Native's packager doesn't work when the RN package exists in multiple places in anode_modulesdirectory. For the purposes of tests, some packages listreact-nativeas a devDependency."testURL": "http://localhost/"if we want to uselocalStoragein tests.