react-native-radar icon indicating copy to clipboard operation
react-native-radar copied to clipboard

Expo plugin

Open KennyHuRadar opened this issue 1 year ago • 5 comments

This PR introduces the following:

  1. expo plugins for react-native-radar
  2. example app that uses continuous native generation that tests correctness of expo plugin. Example app has been bumped to expo SDK 50.
  3. updated CI that checks correctness of plugins.

Note: The new example app opts to comment out the code for map libre's dependency and the map. This is due to the fact that there are some issues with the current map-libre's expo plugin. We are opting for our CI to test our code and expo plugin instead of map-libre's. That said, care should be taken when we are updating our RN map UI export and manual QA should be undertaken.

Docs: https://github.com/radarlabs/documentation/pull/451

For this review, there seems to be a lot of lines of code changed but they are mostly auto-generated code from a new expo example app. The files that were modified within the example app are the package.json, app.json, App.tsx and exampleButton.js

KennyHuRadar avatar Mar 06 '24 20:03 KennyHuRadar

@KennyHuRadar do you know if this will be merged anytime soon?

coreybrown89 avatar Jun 19 '24 00:06 coreybrown89

@KennyHuRadar do you know if this will be merged anytime soon?

Yeah this PR did kind of get buried, @lmeier lets revisit this when you get back.

KennyHuRadar avatar Jun 24 '24 13:06 KennyHuRadar

If we are using expo managed build, we could consider removing android/ios in example app from git. Or we can keep it in as an example of what those files should look like for none-managed projects. (We can only track a subset of the files that are important, like the main .java/kt, Objective-C, manifests, and leave the image assets out)

ShiCheng-Lu avatar Jul 02 '24 15:07 ShiCheng-Lu

If we are using expo managed build, we could consider removing android/ios in example app from git. Or we can keep it in as an example of what those files should look like for none-managed projects. (We can only track a subset of the files that are important, like the main .java/kt, Objective-C, manifests, and leave the image assets out)

A managed workflow will indeed relegate those native code to auto-generated code.

Flip side is that apps in the wild may be running without expo or use expo's bare workflow like we were previously doing. Removing our native folders may leave them out in the cold.

KennyHuRadar avatar Jul 02 '24 15:07 KennyHuRadar

verified working with local install, we can make a pre-release just in case.

ShiCheng-Lu avatar Jul 02 '24 16:07 ShiCheng-Lu

Will it be a requirement to add this plugin to the expo config?

"plugins": [
      [
        "react-native-radar",
        {
          "iosFraud": true,
          "iosNSLocationAlwaysAndWhenInUseUsageDescription": "test value",
          "iosBackgroundMode": true,
          "androidFraud": true,
          "androidBackgroundPermission": true,
          "androidFineLocationPermission": true
        }
      ],
      ["@maplibre/maplibre-react-native"]
    ]

Perhaps some documentation would be good for these options (and defaults) and whether maplibre is a requirement too? Thanks

ChromeQ avatar Jul 26 '24 01:07 ChromeQ

Will it be a requirement to add this plugin to the expo config?

"plugins": [
      [
        "react-native-radar",
        {
          "iosFraud": true,
          "iosNSLocationAlwaysAndWhenInUseUsageDescription": "test value",
          "iosBackgroundMode": true,
          "androidFraud": true,
          "androidBackgroundPermission": true,
          "androidFineLocationPermission": true
        }
      ],
      ["@maplibre/maplibre-react-native"]
    ]

Perhaps some documentation would be good for these options (and defaults) and whether maplibre is a requirement too? Thanks

No it will not be, it is only needed if you are also using Radar Maps. We have a documentation PR that will go live once we include this change in an upcoming release.

KennyHuRadar avatar Jul 26 '24 13:07 KennyHuRadar