-
Notifications
You must be signed in to change notification settings - Fork 60
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
fix: RNPermissions error in Playground #662
base: master
Are you sure you want to change the base?
fix: RNPermissions error in Playground #662
Conversation
@@ -43,7 +43,7 @@ | |||
"@types/jest": "^25.2.1", | |||
"@types/react": "^16.9.32", | |||
"@types/react-native": "^0.63.1", | |||
"@types/react-native-permissions": "^2.0.0", | |||
"react-native-permissions": "^4.1.5", |
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.
Seems like we shouldn't need this if the types are already included in the package. Or were they only included as of 4.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.
So for some reason when running in new architecture, the @babylonjs/react-native
package needs the implementation of react-native-permissions
in it's corresponding node_modules
folder without it the mentioned error gets thrown. Also types are included as of 3+
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.
It looks like it's a standard practice to add your peer dependencies as dev dependencies in development.
Here is an example of react native builder bob (popular create-react-native-library) which adds react-native
as dev dependency: https://github.com/callstack/react-native-builder-bob/blob/9a025b2a89c7790b74862bf95ccab4b6a388e2cb/packages/create-react-native-library/templates/common/%24package.json#L90
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 is so much changed in package-lock? Did you upgrade a bunch of other packages as well? If so I think that is fine if everything is working, just trying to understand.
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.
Weird, I will look into this, as I only updated this package
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.
@ryantrem I updated other dependencies now, looks like react-native-permissions had peer dependency issue with @types/react 16.9 which should be updated... :)
I upgraded this for all 3 packages, just to make sure you can checkout my branch and try installing dependencies too but there shouldn't be any peer deps issues now.
e8e0e07
to
4a8d3cd
Compare
4a8d3cd
to
24b19e3
Compare
This PR fixes
RNPermissions
error in Playground. Currently, when launching the BRNPlayground from master the below error is thrown:This PR addresses the issue.
Screenshots
Documentation
N/A
Testing
Launch test app from master