-
Notifications
You must be signed in to change notification settings - Fork 85
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
New Example - PurchaseTester TypeScript #320
Conversation
e679d76
to
2d8685e
Compare
.circleci/config.yml
Outdated
- run: | ||
command: >- | ||
yarn --cwd examples/purchaseTesterTypescript && yarn install --non-interactive --cache-folder /tmp/yarn | ||
name: Yarn Install (examples/purchaseTesterTypescript) | ||
# - save_cache: | ||
# key: > | ||
# yarn-cache-{{ arch }}-{{ checksum "~/.tmp/checksumfiles/package.json" |
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.
Looks like someone managed to fix it? react-native-community/react-native-circleci-orb#66 (comment) It would be amazing you have some time and can try re-enabling this since running these jobs take forever and this can save us some time.
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.
derived_data_path: ~/DerivedData | ||
project_type: workspace | ||
project_path: examples/purchaseTesterTypescript/ios/PurchaseTester.xcworkspace | ||
scheme: PurchaseTester |
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.
I wonder if we need to build both apps. Running one takes already very long. I am all for just doing one, preferably the new one you just created. Also, I understand we want to keep both tester apps because one is JS and the other one is TS, but the JS one is so bad... maybe we could just simplify it even more than it is now and keep it as a barebones app where we can test the API. What do you think?
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.
Yeah yeah, this takes super long 😱 I agree that running the typescript one probably makes the most sense. I think more issues could come out of that one than the javascript one.
maybe we could just simplify it even more than it is now and keep it as a barebones app where we can test the API. What do you think?
Are you referring to the javascript one? ☝️
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.
@vegaro do you mean turning the javascript app into an API tester?
const [state, setState] = useState(initialState); | ||
|
||
// The functional way of componentDidMount | ||
useEffect(() => { |
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.
👨🍳
const makePurchase = async () => { | ||
Alert.prompt( | ||
"Purchase Product", | ||
"Enter Product ID for purchasing", |
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.
Oh this is great
"@react-navigation/native-stack": "^6.2.5", | ||
"react": "17.0.2", | ||
"react-native": "0.66.4", | ||
"react-native-purchases": "^4.5.0", |
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.
Since this is a tester app, I think it should point to the local version of the plugin, that way it can be used while developing a new feature to test the TypeScript APIs. PurchaseTester apps don't represent the intended usage of our plugin and we use them for development. We have MagicWeather as an example of how a real app looks and that one points to the npm package (although we always forget to update it and it's still pointing to 4.0.0 😮💨 ).
I don't remember how we set it up, but if you look in the purchaseTester/package.json, there's no reference to react-native-purchases
. The package is linked tho (take a look at the podfile and the settings.gradle). So I think it's getting the JS files in metro.config.js
and babel.config.js
, because we have const pak = require('../../package.json');
in those files. This https://arjun30.medium.com/manually-adding-local-library-project-to-reactnative-app-711818dafb79 may be what we did 🤔 , but it took as a while to get to a working setup that didn't involve re-adding the plugin to the sample every time the plugin code is updated. Let me know if you have any questions regarding this.
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.
@vegaro Thoughts on doing this in a separate PR? 😁 I started messing around with this and it got weird 😛 I think it would be nice to see a separate diff and for historical purposes 🤷♂️
Scratch that. I'm so close to getting this working 🤔
awesome!! question: do we need to have the package-lock.json file versioned? |
We do not need to have that in there 😇 I thought I deleted that but I guess not 😛 |
c3e669a
to
958892f
Compare
958892f
to
f238ce8
Compare
b00cb33
to
d45adc2
Compare
2109d4f
to
3e0890b
Compare
3e0890b
to
74810a5
Compare
The ReactNative orb changes got merged in 💪 react-native-community/react-native-circleci-orb#124
|
- save_cache: | ||
paths: | ||
- examples/purchaseTester/ios/Pods | ||
key: cache-pods-{{ checksum "examples/purchaseTester/ios/Podfile.lock" }}-{{ .Environment.CACHE_VERSION }} | ||
|
||
jobs: | ||
analyse_js: |
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.
should we rename this now that it's not just js?
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.
I started to rename this to just analyse
but it then made it unclear what layer it was analyzing 🤔 Even though its not 100% correct and TypeScript is a superset of JavaScript, I think it feels less worse this way 😅
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.
how about analyze_ts_and_js
or analyze_ts_js
? I usually go overly explicit even if it becomes verbose
@@ -9,6 +9,7 @@ node_modules/ | |||
npm-debug.log | |||
yarn-error.log | |||
**/yarn.lock | |||
**/package-lock.json |
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.
🔥
@@ -0,0 +1,6 @@ | |||
|
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.
idk if this kinda file is common knowledge for RN developers (and maybe ignore me if so), but could we add a comment on what this does for us?
const Stack = createNativeStackNavigator(); | ||
|
||
const App = () => { | ||
const isSetup = () => { |
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.
this is nitpicky, but could we rename to something like hasKeys
? just since setup is now a little overloaded... isSetup could also mean whether Purchases is configured
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.
This is a much better name 😊 Love it!
return !isSetup() ? ( | ||
<SafeAreaView> | ||
<Text style={{margin: 20, textAlign: 'center'}}> | ||
Update RevenueCat API Keys in APIKeys.tsx | ||
</Text> | ||
</SafeAreaView> | ||
) : ( | ||
<NavigationContainer> | ||
<Stack.Navigator initialRouteName="Home"> | ||
<Stack.Screen | ||
name="Home" | ||
component={HomeScreen} | ||
options={{ title: 'PurchaseTester' }} | ||
/> | ||
<Stack.Screen name="CustomerInfo" component={CustomerInfoScreen} /> | ||
<Stack.Screen name="OfferingDetail" component={OfferingDetailScreen} /> | ||
</Stack.Navigator> | ||
</NavigationContainer> |
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.
i love this, we should definitely consider in the other platforms. of course well get a configuration error, but if you can't view debug logs you might not know
@@ -0,0 +1,55 @@ | |||
# To learn about Buck see [Docs](https://buckbuild.com/). |
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.
oh, ignore above comment about the buck file :)
* This works so much better for local testing * Updated comments to be more helpful * Fixed typo * Need to await the initial fetchData
I think this is ready for the final review if anybody has time 😊 |
@@ -26,12 +26,12 @@ import { SafeAreaView } from 'react-native-safe-area-context'; | |||
const Stack = createNativeStackNavigator(); | |||
|
|||
const App = () => { | |||
const isSetup = () => { | |||
const hasKeys = () => { |
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.
👍
}), | ||
}, | ||
}; | ||
// IMPORTANT |
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 mind updating our notion docs here once this is merged?
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.
🥳 thanks for not only making a 🔥 sample app but for improving our development flow! i'll be dibs-ing some react-native tickets soon to get to really try it out :)
🎈🐐 |
See #320 for the replacement.
See #320 for the replacement.
Description
This adds a new ReactNative TypeScript example. It can be used to test almost all of features from the ReactNative Purchases SDK. There are a few screens in this app:
Other Bits
package.json
so both projects work withyarn bootstrap
How to use
APIKeys.tsx
Requirements
Show RevenueCat logs on screen (you can use Purchases.setLogHandler)(not possible with SDK)Logging backend requests / responses(not possible with SDK I think)Demo
iOS (Simulator)
RocketSim_Recording_iPod_touch_.7th_generation._2021-12-28_09.54.47.mp4
Android (Emulator)
Screen.Recording.2021-12-28.at.5.27.04.PM.mov