feat(macos): support react-native-macos #1002

shirakaba · 2024-07-22T05:22:03Z

Description

Steps to verify

Follow the updated setup instructions in examples/demo/README.md to run the demo app. See the following screenshots for expected behaviour with the two providers:

changeset-bot · 2024-07-22T05:22:08Z

🦋 Changeset detected

Latest commit: b3da68b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
react-native-app-auth	Minor
rnaa-demo	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2024-07-22T05:22:08Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-native-app-auth	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 30, 2024 6:02am

examples/demo/metro.config.js

examples/demo/package.json

shirakaba · 2024-07-22T05:29:46Z

packages/react-native-app-auth/ios/RNAppAuth.m


 #if TARGET_OS_MACCATALYST
    id<OIDExternalUserAgent> externalUserAgent = nil;
 #elif TARGET_OS_IOS
    id<OIDExternalUserAgent> externalUserAgent = iosCustomBrowser != nil ? [self getCustomBrowser: iosCustomBrowser] : [self getExternalUserAgentWithPresentingViewController:presentingViewController
                                                                                                                                    prefersEphemeralSession:prefersEphemeralSession];
+#elif TARGET_OS_OSX
+    id<OIDExternalUserAgent> externalUserAgent = nil;


I was unsure about this externalUserAgent = nil, but it does turn out that under-the-hood for the APIs we're calling, the AppAuth library makes an appropriate OIDExternalUserAgentMac for you so there's no need to make one yourself.

shirakaba · 2024-07-22T05:30:12Z

packages/react-native-app-auth/ios/RNAppAuth.m

+#if TARGET_OS_IOS || TARGET_OS_MACCATALYST
                                                          [UIApplication.sharedApplication endBackgroundTask:rnAppAuthTaskId];
                                                          rnAppAuthTaskId = UIBackgroundTaskInvalid;
+#elif TARGET_OS_OSX
+                                                          // Brings this app to the foreground.
+                                                          [[NSRunningApplication currentApplication] activateWithOptions:(NSApplicationActivateAllWindows)];
+#endif


macOS AppKit apps don't get backgrounded, so we don't need to declare background tasks (but do benefit from refocusing the app).

shirakaba · 2024-07-22T05:34:27Z

packages/react-native-app-auth/react-native-app-auth.podspec

-  s.platform     = :ios, '10.0'
+  s.ios.deployment_target = '10.0'
+  s.osx.deployment_target = '10.15'


Technically, AppAuth lets us deploy down to the following versions:

s.ios.deployment_target = "9.0" s.osx.deployment_target = "10.12"

I've chosen 10.15 for this library just to align with the react-native-macos template's minimum deployment target.

packages/react-native-app-auth/ios/RNAppAuth.m

shirakaba · 2024-07-22T23:59:08Z

Thanks @Saadnajmi for your great guidance! I'll address those aspects once I'm free (think today I'll be working on other things, however). As we discussed on Twitter, react-native-test-app may be the way forwards for integrating an out-of-tree-platform's example app into this repo, and rnx-kit may be able to help out with the Metro config.

shirakaba · 2024-07-27T02:51:10Z

@Saadnajmi I've now migrated the PR to use react-native-test-app, which resolves my issues with the Metro bundler. I've also aligned to a common minor of React Native, and have addressed all the ifdefs.

However, the demo app requires a custom AppDelegate to make it conform to RNAppAuthAuthorizationFlowManager. How would I set that up?

Saadnajmi · 2024-07-27T06:20:57Z

@Saadnajmi I've now migrated the PR to use react-native-test-app, which resolves my issues with the Metro bundler. I've also aligned to a common minor of React Native, and have addressed all the ifdefs.

However, the demo app requires a custom AppDelegate to make it conform to RNAppAuthAuthorizationFlowManager. How would I set that up?

I don't think RNTA supports custom App Delegates, so that would be a feature request (@tido64 fyi). It does support expo config plugins, does that let you modify the app delegate? https://github.com/microsoft/react-native-test-app/wiki/Config-Plugins

Though I'm realizing for macOS, that means you need 0.74 😅

shirakaba · 2024-07-27T07:18:11Z

Unfortunately the one implementing Expo config plugins support for react-native-macos is none other than myself, so I can report that it’s not ready yet 😅

I’ve actually already completed the implementation of Expo config plugins, but need to rewrite the tests, which I was hoping to get at this evening. Still though, it would depend on Expo reviewing and merging it, and even then would only make it into SDK 51 (which expects RN 0.74 🥲) or later. So I think we’d either need a new feature in RNTestApp or go without it in this case.

shirakaba · 2024-07-27T14:17:08Z

Okay, I've ripped out RNTA again and now have a mix of [email protected] and [email protected].

I've tried raising react-native to v0.73 to get them both on v0.73 and couldn't get iOS to build (even when using the react-native iOS template from React Native Community CLI). Some issue to do with examples/demo/node_modules/@react-native/codegen/lib/generators/modules/GenerateModuleObjCpp/serializeMethod.js#495:

"NativeAppState.getConstants()" may only return an object literal: {...}.

I've also tried dropping react-native-macos to v0.72 to get them both on v0.72 and couldn't get macOS to build (tried generating the react-native-macos template for v0.72 and the CLI threw errors, so couldn't proceed). Some genericNodeError from react-native-macos-init with no information:

The react-native-macos-init error

scratch/RNSACExample npx react-native-macos-init --version 0.72.21
Reading application name from package.json…
Reading react-native version from node_modules…
Reading react-native-macos version from node_modules…
Installing [email protected]…
Command failed: yarn add -s "[email protected]" Error: Command failed: yarn add -s "[email protected]"
    at genericNodeError (node:internal/errors:984:15)
    at wrappedFn (node:internal/errors:538:14)
    at checkExecSyncError (node:child_process:890:11)
    at Object.execSync (node:child_process:962:15)
    at Object.<anonymous> (/Users/jamie/.npm/_npx/20f1505fae2cef5b/node_modules/react-native-macos-init/lib-commonjs/cli.js:247:29)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/jamie/.npm/_npx/20f1505fae2cef5b/node_modules/react-native-macos-init/lib-commonjs/cli.js:10:58)
    at runNextTicks (node:internal/process/task_queues:60:5)
    at process.processImmediate (node:internal/timers:454:9) {
  status: 1,
  signal: null,
  output: [
    null,
    <Buffer 1b 5b 33 31 6d 1b 5b 31 6d 55 6e 6b 6e 6f 77 6e 20 53 79 6e 74 61 78 20 45 72 72 6f 72 1b 5b 32 32 6d 1b 5b 33 39 6d 3a 20 55 6e 73 75 70 70 6f 72 74 ... 183 more bytes>,
    <Buffer >
  ],
  pid: 3382,
  stdout: <Buffer 1b 5b 33 31 6d 1b 5b 31 6d 55 6e 6b 6e 6f 77 6e 20 53 79 6e 74 61 78 20 45 72 72 6f 72 1b 5b 32 32 6d 1b 5b 33 39 6d 3a 20 55 6e 73 75 70 70 6f 72 74 ... 183 more bytes>,
  stderr: <Buffer >
}

This is the best I can do! Would be able to get them both on the same version if RNTA supported custom AppDelegates, however.

tido64 · 2024-07-29T16:10:35Z

Would be able to get them both on the same version if RNTA supported custom AppDelegates, however.

I don't know if this helps, but I've had this branch for a while that I've just submitted: microsoft/react-native-test-app#2160

shirakaba · 2024-08-05T09:52:21Z

@tido64 Thanks so much for that PR; it should make everything easier! Sorry I wasn't able to review any closer – I had my hands full last week. I'll try it out once I get back to this PR.

robwalkerco

Thanks for the great PR ❤️

I've spotted a few very minor issues that would be great if you could address before we merge.

robwalkerco · 2024-08-15T13:46:26Z

examples/demo/ios/Podfile.lock

The current version of CocoaPods is 1.15.2 - https://github.com/CocoaPods/CocoaPods/releases

Can this change be reverted, or is there a specific reason for this change?

robwalkerco · 2024-08-15T14:47:26Z

examples/demo/README.md

It would be very helpful if you could add/update any setup instructions to cover macOS as well on docs/docs/introduction.md and README.md.

robwalkerco · 2024-08-15T15:59:04Z

examples/demo/macos/Podfile.lock

It would be great if this could also use COCOAPODS: 1.15.2 instead of 1.14.3

shirakaba · 2024-08-16T04:31:15Z

@robwalkerco Thank you for the review! Pending tasks in my mind:

~~Adopt React Native Test App again (now that it supports custom AppDelegates)~~
~~Update to React Native v0.74 (as it recently became available for React Native macOS)~~
Use the same minor version of React Native for both react-native and react-native-macos
As per review: Use CocoaPods 1.15.2
As per review: Add any setup instructions to cover macOS into docs/docs/introduction.md and README.md.

I've been a little burnt out lately so may not be able to jump straight on this but do hope to push this over the line soon.

carbonrobot · 2024-08-16T12:04:27Z

.yarnrc

+# yarn lockfile v1
+
+
+yarn-path ".yarn/releases/yarn-1.19.1.cjs"


If we want to use a non-global version of Yarn, we should look at enabling corepack instead of checking in the binary. Since this is unrelated to this specific PR, it would be best to revert this change and open a separate PR for this specifically.

I was actually hoping to just install using global Yarn v1 here, but installation always failed with the following error:

expected workspace package to exist for "eslint"

I looked up the error and it recommended this as the fix. If you know another way to get yarn to successfully install this repo's packages, I'd be happy to implement that.

I managed to successfully install with global Yarn v1, so I've removed this.

shirakaba · 2024-08-18T05:29:06Z

I've done a little more work on this to see if this can be migrated to React Native Test App, which would be helpful for adding Windows support in future.

@tido64 I'm struggling to use macos.withAppDelegate() to set up the necessary custom AppDelegate. We need to do two things:

Make AppDelegate conform to RNAppAuthAuthorizationFlowManager. This is possible by referencing the following examples/demo/config-plugin.js via "plugins": ["./config-plugin.js"] in examples/demo/app.json:

const { createRunOncePlugin, } = require("@expo/config-plugins");
const { mergeContents } = require("@expo/config-plugins/build/utils/generateCode");
const { macos } = require("react-native-test-app/plugins/index");

function withCustomAppDelegate(config) {
  return macos.withAppDelegate(config, (config) => {
    // See:
    // - examples/demo/node_modules/react-native-test-app/macos/ReactTestApp/AppDelegate.swift
    // - https://github.com/microsoft/react-native-test-app/pull/2160#issue-2435799831
    config.modResults.contents = mergeContents({
      tag: "config-plugin.js",
      src: config.modResults.contents,
      newSrc: "extension AppDelegate: RNAppAuthAuthorizationFlowManager {}",
      anchor: /extension AppDelegate {/,
      offset: 0,
      comment: "//",
    }).contents;

    return config;
  });
}

module.exports = createRunOncePlugin(
  withCustomAppDelegate,
  "test-plugin.js",
  "UNVERSIONED"
);

Express the following Obj-C as Swift in AppDelegate.swift:

#import <React/RCTLinkingManager.h>

// ... a few lines later:

- (void)applicationWillFinishLaunching:(NSNotification *)notification {
  [[NSAppleEventManager sharedAppleEventManager] setEventHandler:self
                           andSelector:@selector(getURL:withReplyEvent:)
                         forEventClass:kInternetEventClass
                            andEventID:kAEGetURL];
}

- (void)getURL:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEventDescriptor *)reply
{
  NSString* urlString = [[event paramDescriptorForKeyword:keyDirectObject] stringValue];
  NSURL *url = [NSURL URLWithString:urlString];

  if ([self.authorizationFlowManagerDelegate resumeExternalUserAgentFlowWithURL:url]) {
    return;
  }
  
  [RCTLinkingManager getUrlEventHandler:event withReplyEvent:reply];
}

... Unfortunately, the current AppDelegate has multiple occurences of the string "applicationWillFinishLaunching", so it is impossible to come up with a non-fragile RegExp pattern to use with it (particularly as the config plugin does not support multi-line regexes). The Expo Config AppDelegate Plugin moreover seems to no-op if there's more than one match.

Largely, this is down to the mergeContents() method from Expo Config Plugins being a very limited API to begin with. I suppose there's no need to use mergeContents(), actually – maybe we can just do a bit of manual string manipulation ourselves inside withAppDelegate 🤔

shirakaba · 2024-08-18T05:33:23Z

@robwalkerco I'm proceeding to try to get this onto React Native Test App so that adding support for Windows in future will be simpler and to improve long-term maintainability (we'd be able to just bump the version of RNTA to upgrade all example apps). Would you be happy with me migrating the repo to use RNTA as part of this PR, or should it be a follow-up PR?

(Noting that I wasn't having much luck getting this working without RNTA – testing the react-native and react-native-macos example apps required manually flipping a variable in metro.config.js depending on which one wanted to bundle at the given time)

tido64 · 2024-08-19T09:59:12Z

... Unfortunately, the current AppDelegate has multiple occurences of the string "applicationWillFinishLaunching", so it is impossible to come up with a non-fragile RegExp pattern to use with it (particularly as the config plugin does not support multi-line regexes). The Expo Config AppDelegate Plugin moreover seems to no-op if there's more than one match.

I added some anchor points. I hope this helps: microsoft/react-native-test-app#2188

robwalkerco · 2024-08-19T15:09:31Z

Would you be happy with me migrating the repo to use RNTA as part of this PR, or should it be a follow-up PR?

@shirakaba It would be preferable if adding RNTA could be handled in a separate PR. That should also help to get this initial macOS PR merged more quickly for you.

… exist for "eslint"' error yarnpkg/yarn#8405

FIXME: need to ask how to customise the AppDelegate as in the previous commit, as it's complaining about: Thread 1: "<ReactTestApp.AppDelegate: 0x10a3e7b50> does not conform to RNAppAuthAuthorizationFlowManager" And will likely need to do similar customisation for Android.

Based on these: facebook/react-native#42278

This reverts commit ce193fc.

shirakaba · 2024-10-30T06:07:54Z

@robwalkerco I've:

rebased onto main
matched the minor version of react-native (v0.72) used in this repo
removed RNTA
removed the local yarn
removed the need for a applePlatform = "macos" config flag in metro.config.js so that we can use the same bundler to run all platforms at once
updated the README.md
confirmed the app still works on iOS

Is there any blocker to merging this now?

vercel bot deployed to Preview July 22, 2024 05:22 View deployment

shirakaba force-pushed the feat/macos/support-react-native-macos branch from 4e9d4ce to 259809c Compare July 22, 2024 05:24

vercel bot deployed to Preview July 22, 2024 05:24 View deployment

shirakaba commented Jul 22, 2024

View reviewed changes

shirakaba changed the title ~~Feat/macos/support react native macos~~ feat(macos): support react-native-macos Jul 22, 2024

shirakaba commented Jul 22, 2024

View reviewed changes

packages/react-native-app-auth/ios/RNAppAuth.m Outdated Show resolved Hide resolved

Saadnajmi reviewed Jul 22, 2024

View reviewed changes

packages/react-native-app-auth/ios/RNAppAuth.m Outdated Show resolved Hide resolved

shirakaba mentioned this pull request Jul 24, 2024

feat: add react-native-macos support th3rdwave/react-native-safe-area-context#513

Merged

2 tasks

shirakaba force-pushed the feat/macos/support-react-native-macos branch from 259809c to 8ec536a Compare July 27, 2024 02:45

vercel bot deployed to Preview July 27, 2024 02:45 View deployment

shirakaba marked this pull request as draft July 27, 2024 02:51

shirakaba force-pushed the feat/macos/support-react-native-macos branch from 8ec536a to ce193fc Compare July 27, 2024 02:55

vercel bot deployed to Preview July 27, 2024 02:55 View deployment

vercel bot deployed to Preview July 27, 2024 03:55 View deployment

vercel bot deployed to Preview July 27, 2024 12:46 View deployment

vercel bot deployed to Preview July 27, 2024 14:08 View deployment

shirakaba marked this pull request as ready for review July 27, 2024 14:17

shirakaba requested a review from Saadnajmi July 27, 2024 14:18

robwalkerco reviewed Aug 15, 2024

View reviewed changes

carbonrobot requested changes Aug 16, 2024

View reviewed changes

vercel bot deployed to Preview August 18, 2024 03:04 View deployment

shirakaba force-pushed the feat/macos/support-react-native-macos branch from 38d7a63 to 5ca0d4a Compare October 30, 2024 05:15

vercel bot deployed to Preview October 30, 2024 05:37 View deployment

shirakaba force-pushed the feat/macos/support-react-native-macos branch from 3248a96 to 0f7670c Compare October 30, 2024 05:46

vercel bot deployed to Preview October 30, 2024 05:47 View deployment

shirakaba added 12 commits October 30, 2024 14:50

feat(macos): implement macOS support

25abcf6

fix: stop yarn install from throwing 'expected workspace package to…

c2d4ee4

… exist for "eslint"' error yarnpkg/yarn#8405

feat(macos): add react-native-macos demo app

ee514fc

chore: add changeset

a8190d3

fix(ios,macos): amend platform ifdefs

d5b8c28

Based on these: facebook/react-native#42278

Revert "feat: migrate to react-native-test-app"

c41d1ed

This reverts commit ce193fc.

fix: use Metro config from rnx-kit

d4ba607

fix(macos): ignore .xcode.env.local

bbbae81

chore: update Podfile.lock

fb4b005

fix: reinit app with [email protected]

d7f3c41

fix: re-apply demo-specific code

a9ddf95

shirakaba force-pushed the feat/macos/support-react-native-macos branch from 0f7670c to a9ddf95 Compare October 30, 2024 05:51

vercel bot deployed to Preview October 30, 2024 05:52 View deployment

shirakaba added 3 commits October 30, 2024 14:57

fix: add missed items from rebase

1dd0b44

fix: update instructions

85b1e5d

fix: remove local yarn

b3da68b

vercel bot deployed to Preview October 30, 2024 06:02 View deployment

shirakaba requested review from robwalkerco and carbonrobot October 30, 2024 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(macos): support react-native-macos #1002

feat(macos): support react-native-macos #1002

shirakaba commented Jul 22, 2024 •

edited

Loading

changeset-bot bot commented Jul 22, 2024 •

edited

Loading

vercel bot commented Jul 22, 2024 •

edited

Loading

shirakaba Jul 22, 2024

shirakaba Jul 22, 2024

shirakaba Jul 22, 2024

shirakaba commented Jul 22, 2024 •

edited

Loading

shirakaba commented Jul 27, 2024 •

edited

Loading

Saadnajmi commented Jul 27, 2024

shirakaba commented Jul 27, 2024

shirakaba commented Jul 27, 2024 •

edited

Loading

tido64 commented Jul 29, 2024

shirakaba commented Aug 5, 2024 •

edited

Loading

robwalkerco left a comment

robwalkerco Aug 15, 2024

robwalkerco Aug 15, 2024

robwalkerco Aug 15, 2024

shirakaba commented Aug 16, 2024 •

edited

Loading

carbonrobot Aug 16, 2024

shirakaba Aug 18, 2024 •

edited

Loading

shirakaba Oct 30, 2024

shirakaba commented Aug 18, 2024 •

edited

Loading

shirakaba commented Aug 18, 2024 •

edited

Loading

tido64 commented Aug 19, 2024

robwalkerco commented Aug 19, 2024

shirakaba commented Oct 30, 2024 •

edited

Loading

		# yarn lockfile v1


		yarn-path ".yarn/releases/yarn-1.19.1.cjs"

feat(macos): support react-native-macos #1002

Are you sure you want to change the base?

feat(macos): support react-native-macos #1002

Conversation

shirakaba commented Jul 22, 2024 • edited Loading

Description

Steps to verify

changeset-bot bot commented Jul 22, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shirakaba commented Jul 22, 2024 • edited Loading

shirakaba commented Jul 27, 2024 • edited Loading

Saadnajmi commented Jul 27, 2024

shirakaba commented Jul 27, 2024

shirakaba commented Jul 27, 2024 • edited Loading

tido64 commented Jul 29, 2024

shirakaba commented Aug 5, 2024 • edited Loading

robwalkerco left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shirakaba commented Aug 16, 2024 • edited Loading

Choose a reason for hiding this comment

shirakaba Aug 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shirakaba commented Aug 18, 2024 • edited Loading

shirakaba commented Aug 18, 2024 • edited Loading

tido64 commented Aug 19, 2024

robwalkerco commented Aug 19, 2024

shirakaba commented Oct 30, 2024 • edited Loading

shirakaba commented Jul 22, 2024 •

edited

Loading

changeset-bot bot commented Jul 22, 2024 •

edited

Loading

vercel bot commented Jul 22, 2024 •

edited

Loading

shirakaba commented Jul 22, 2024 •

edited

Loading

shirakaba commented Jul 27, 2024 •

edited

Loading

shirakaba commented Jul 27, 2024 •

edited

Loading

shirakaba commented Aug 5, 2024 •

edited

Loading

shirakaba commented Aug 16, 2024 •

edited

Loading

shirakaba Aug 18, 2024 •

edited

Loading

shirakaba commented Aug 18, 2024 •

edited

Loading

shirakaba commented Aug 18, 2024 •

edited

Loading

shirakaba commented Oct 30, 2024 •

edited

Loading