zoontek / react-native-permissions

An unified permissions API for React Native on iOS, Android and Windows.
MIT License
4.11k stars 837 forks source link

feat: setup expo config plugin for ios permissions setup #834

Closed fobos531 closed 10 months ago

fobos531 commented 11 months ago

Summary

Hello @zoontek !

Thank you for this amazing library, to my knowledge this is the sole library which provides a built-in way to check for Bluetooth permissions on iOS and I use that a lot, so this is very useful to me!

I would like to contribute back to this library by providing an Expo config plugin which simplifies the iOS permission setup configuration. The way it is set up is that Expo users would use your library like this:

plugins: [
    [
        'react-native-permissions',
          {
            iosPermissions: ['Bluetooth', 'Calendars', 'LocationWhenInUse'] // etc..
          },
    ]
]

Essentially, what it does it automates the steps 1 & 2 of the iOS setup (which are the most "tedious") from your README. The rest of the setup is supported out-of-the-box by Expo anyways.

Test Plan

I would wholeheartedly like to test but I'm running into a few issues that I believe are NOT caused by me adding this integration. I'll list them here, so I hope you could provide insight into why they're happening since I'm not fully familiar with this repo.

  1. When yarn prepack (i.e. bob build) runs, I'm met with this error:
$ bob build
/Users/jakovglavina/OSS/react-native-permissions/node_modules/cliui/build/index.cjs:291
const stringWidth = require('string-width');
                    ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/jakovglavina/OSS/react-native-permissions/node_modules/string-width/index.js from /Users/jakovglavina/OSS/react-native-permissions/node_modules/cliui/build/index.cjs not supported.
Instead change the require of index.js in /Users/jakovglavina/OSS/react-native-permissions/node_modules/cliui/build/index.cjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/Users/jakovglavina/OSS/react-native-permissions/node_modules/cliui/build/index.cjs:291:21)
    at Object.<anonymous> (/Users/jakovglavina/OSS/react-native-permissions/node_modules/yargs/build/index.cjs:1:60678)
    at Object.<anonymous> (/Users/jakovglavina/OSS/react-native-permissions/node_modules/yargs/index.cjs:5:30)
    at Object.<anonymous> (/Users/jakovglavina/OSS/react-native-permissions/node_modules/react-native-builder-bob/lib/index.js:7:37)
    at Object.<anonymous> (/Users/jakovglavina/OSS/react-native-permissions/node_modules/react-native-builder-bob/bin/bob:4:1) {
  code: 'ERR_REQUIRE_ESM'
}

It looks like this is caused by one of the dependencies (builder bob?) - any idea how exactly to resolve this? This is blocking me from testing it by either installing it in the example project or by publishing to NPM and using it then. If we can figure out how to fix this, I'm hoping I can get it running and verify that the changes specified in the config plugin are actually properly applied.

Also, I'm getting this ESLint issue which I'm not sure how to fix:

CleanShot 2023-12-23 at 18 10 36@2x

I see that you have a fairly involved ESLint config file, so I'm not sure what is necessary to do to ensure that ESLint is happy with the app.plugin.js being present in the root directory of this project.

When this is resolved, I can add docs to the README to inform users that this feature is available for them to use.

I hope you can take this pull request into consideration.

Thank you!

What's required for testing (prerequisites)?

Expo project.

What are the steps to test it (after prerequisites)?

Add this package in an Expo project, use the Expo config plugin like so:

plugins: [
    [
        'react-native-permissions',
          {
            iosPermissions: ['Bluetooth']
          },
    ]
]

Observe that the resulting Podfile contains the same changes that it would contain if the setup was done manually.

Compatibility

OS Implemented
iOS
Android

Checklist

zoontek commented 11 months ago

Hey! A few things:

"devDependencies": {
  "@babel/core": "^7.20.0",
  "@babel/preset-env": "^7.20.0",
+ "@expo/config-plugins": "^7.2.5",
  "@types/react": "^18.2.45",
  "@typescript-eslint/eslint-plugin": "^6.14.0",
  "@typescript-eslint/parser": "^6.14.0",
  "eslint": "^8.55.0",
  "eslint-plugin-react": "^7.33.2",
  "eslint-plugin-react-hooks": "^4.6.0",
- "expo": "^49.0.21",
  "expo-module-scripts": "^3.1.0",
  "lint-staged": "^15.2.0",
  "prettier": "^3.1.1",
  "prettier-plugin-organize-imports": "^3.2.4",
  "react": "^18.2.0",
  "react-native": "^0.73.0",
  "react-native-builder-bob": "^0.23.2",
  "typescript": "^5.3.3"
}
import {ConfigPlugin, withDangerousMod} from '@expo/config-plugins';
import {mergeContents} from '@expo/config-plugins/build/utils/generateCode';
import fs from 'fs';
import path from 'path';

type Props = {
  iosPermissions?: Array<
    | 'AppTrackingTransparency'
    | 'Bluetooth'
    | 'Calendars'
    | 'CalendarsWriteOnly'
    | 'Camera'
    | 'Contacts'
    | 'FaceID'
    | 'LocationAccuracy'
    | 'LocationAlways'
    | 'LocationWhileInUse'
    | 'MediaLibrary'
    | 'Microphone'
    | 'Motion'
    | 'Notifications'
    | 'PhotoLibrary'
    | 'PhotoLibraryAddOnly'
    | 'Reminders'
    | 'Siri'
    | 'SpeechRecognition'
    | 'StoreKit'
  >;
};

const plugin: ConfigPlugin<Props> = (config, {iosPermissions}) => {
  if (iosPermissions == null || iosPermissions.length === 0) {
    return config;
  }

  return withDangerousMod(config, [
    'ios',
    async (config) => {
      const file = path.join(config.modRequest.platformProjectRoot, 'Podfile');
      let contents = await fs.promises.readFile(file, 'utf8');

      contents = mergeContents({
        tag: 'node-require-function',
        src: contents,
        newSrc: `def node_require(script)\n\t# Resolve script with node to allow for hoisting\n\trequire Pod::Executable.execute_command('node', ['-p',\n\t\t"require.resolve(\n\t\t\t'#{script}',\n\t\t\t {paths: [process.argv[1]]},\n\t\t)", __dir__]).strip\nend\n\nnode_require('react-native-permissions/scripts/setup.rb')`,
        anchor: /scripts\/react_native_pods/,
        offset: 1,
        comment: '#',
      }).contents;

      contents = mergeContents({
        tag: 'permissions-array',
        src: contents,
        newSrc: `setup_permissions([${iosPermissions
          .map((permission) => `'${permission}'`)
          .join(',')}])`,
        anchor: /prepare_react_native_project!/,
        offset: 1,
        comment: '#',
      }).contents;

      await fs.promises.writeFile(file, contents, 'utf-8');
      return config;
    },
  ]);
};

export default plugin;

The yarn prepack error does not occurs for me on master, only on your branch, so if you do a proper rebase it should be good.

zoontek commented 11 months ago

expo-module-scripts is also really bloated. Please remove it and replace the plugin tsconfig with this (that's all what it imports when using expo-module-scripts/tsconfig.plugin):

{
  "compilerOptions": {
    "target": "ES2020",
    "lib": ["ES2020"],
    "moduleResolution": "Node16",
    "module": "Node16",

    "strict": true,
    "esModuleInterop": true,
    "declaration": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,

    "rootDir": "src",
    "outDir": "build"
  },
  "include": ["./src"],
  "exclude": ["**/__mocks__/*", "**/__tests__/*"]
}
a-eid commented 10 months ago

@zoontek @fobos531

I used to use version 3 of the lib with expo and it works as expected. all that was needed was adding this package.json hook

   "eas-build-post-install": "react-native setup-ios-permissions && pod-install",
zoontek commented 10 months ago

@a-eid The CLI command has been removed in v4

a-eid commented 10 months ago

@zoontek yeah, can't wait for this PR to get merged so we're able to upgrade to version 4.

zoontek commented 10 months ago

@a-eid I'm waiting for @fobos531 to rebase and perform the requested changes.

fobos531 commented 10 months ago

Hello @zoontek,

apologies, it took me a while to get to it - I also didn't perform the changes exactly how you requested (by rebasing) since I've had some difficulties with getting it to work (still a little unexperienced), but I'm pretty confident the end result is now the same. I was also able to clean-up the plugin a little further. Here: https://www.diffchecker.com/KtEzHQEf/ You can see the diff between the Podfile where the expo config plugin was not applied, and on the right it was applied using the following syntax:

[
      'react-native-permissions',
      {
        iosPermissions: ['Bluetooth'],
      },
    ],

One thing to note is that this is plugin supported on RN 0.72+ only (I didn't account for the require_relative syntax for < 0.72)

Let me know in case you require any other changes from my end.

zoontek commented 10 months ago

Perfect, I merge this is a 4.1.0 for more testing before a release, thanks a lot!

zoontek commented 10 months ago

Done, thanks! https://github.com/zoontek/react-native-permissions/releases/tag/4.1.0