Skip to content

feat: create expo config plugin#313

Merged
thymikee merged 6 commits intocallstack:masterfrom
giautm:expo-plugin
Feb 24, 2022
Merged

feat: create expo config plugin#313
thymikee merged 6 commits intocallstack:masterfrom
giautm:expo-plugin

Conversation

@giautm
Copy link
Copy Markdown
Contributor

@giautm giautm commented Feb 23, 2022

Summary

Fixes #311

Test plan

  • Integration test with the application that config the plugin.

@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 23, 2022

This PR depended on the upstream PR: thebergamo/react-native-fbsdk-next#205

Comment thread .gitignore Outdated
Comment thread README.md
Comment thread README.md Outdated
Comment thread app.plugin.js
Comment thread package.json Outdated
Comment thread plugin/build/withReactNativeFbads.js Outdated
Comment thread plugin/jest.config.js Outdated
Copy link
Copy Markdown
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thanks! I have no experience with expo plugins, hoping to find someone to review this properly

@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 23, 2022

Requests permission to track the user. Requires an NSUserTrackingUsageDescription key in your Info.plist. (See iOS 14 Tracking API)

I found this package also require NSUserTrackingUsageDescription in Info.plist. Do you want the plugin config for the user, or lets it be config by upstream package react-native-fbsdk-next?

@thymikee
Copy link
Copy Markdown
Member

What's easiest for the Expo users?

@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 23, 2022

Because react-native-fbsdk become archived by Facebook and pointed to react-native-fbsdk-next. I think react-native-fbads should support it.

https://github.com/facebookarchive/react-native-fbsdk/blob/master/README.md

@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 23, 2022

What's easiest for the Expo users?

Use react-native-fbads with react-native-fbsdk-next. I guess. 😄

@thymikee
Copy link
Copy Markdown
Member

Oh, then we should point to react-native-fbsdk-next. Sorry, I'm out of the loop with this package 😅

@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 24, 2022

cc @EvanBacon for review

Comment thread plugin/src/withReactNativeFbads.ts Outdated
Comment thread plugin/src/withReactNativeFbads.ts Outdated
Comment thread plugin/src/withReactNativeFbads.ts Outdated
Comment thread plugin/src/withReactNativeFbads.ts Outdated
Copy link
Copy Markdown
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

Looks good, pretty simple config plugin. Opt towards not having props if they're an empty object. Eventually we'll extend autolinking to support indicating that a config plugin has no properties and can be applied automatically when the native module is linked -- this would only work if the plugin has no required properties.

giautm and others added 2 commits February 25, 2022 00:58
Co-authored-by: Evan Bacon <baconbrix@gmail.com>
Copy link
Copy Markdown
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

Looks great, nice work!

Copy link
Copy Markdown
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

Thank you!

@thymikee thymikee merged commit ab5fd00 into callstack:master Feb 24, 2022
@giautm giautm deleted the expo-plugin branch February 24, 2022 22:42
@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 24, 2022

Thank you!

Thank you very much. Please assign any issue that related to Expo for me. I'm ready for help!

@giautm
Copy link
Copy Markdown
Contributor Author

giautm commented Feb 28, 2022

Hi @thymikee, can you make a new release for this update?

@thymikee
Copy link
Copy Markdown
Member

Published. Sorry for the delay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Expo EAS build via plugin config?

3 participants