-
Notifications
You must be signed in to change notification settings - Fork 42
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
Update react-native vulns #397
Conversation
7e22581
to
04a6c52
Compare
configureSpotless(it) | ||
} | ||
|
||
def configureSpotless(Project project) { |
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.
Spotless seems to no longer be compatible with react-native 0.73. This was used for linting, but the android project is autogenerated anyways, so I don't see much value in trying to make linting work again. So, I decided to remove spotless altogether from the sample android app.
@@ -201,7 +201,6 @@ | |||
buildConfigurationList = 13B07F931A680F5B00A75B9A /* Build configuration list for PBXNativeTarget "E2EOktaReactNative" */; |
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.
Changes in this file are from https://docs.expo.dev/bare/upgrade/. I chose from expo 49 to expo 50, and I got the changes needed for the xcode project file
@@ -1,87 +1,74 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> |
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.
This file is unchanged, but XCode decided to reformat it
UIPasteboard.general.string = password | ||
passwordField.doubleTap() | ||
app.menuItems["Paste"].tap() | ||
passwordField.typeText(password) |
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.
app.menuItems["Paste"].tap() wasn't working for me locally. I replaced it with typeText instead, and it seemed to work fine
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.
Sounds good. Some versions of Xcode/the iOS Simulator have trouble with reliably typing text, but I think it improved recently.
@@ -7,7 +7,7 @@ podfile_properties = JSON.parse(File.read(File.join(__dir__, 'Podfile.properties | |||
ENV['RCT_NEW_ARCH_ENABLED'] = podfile_properties['newArchEnabled'] == 'true' ? '1' : '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.
Changes here are from https://docs.expo.dev/bare/upgrade/ Expo 49 -> 50
config = use_native_modules! | ||
use_react_native!(:path => config[:reactNativePath]) | ||
|
||
use_native_modules! |
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.
use_native_modules was causing issues with update to react-native 0.73. Removing that didn't cause any issues, and tests are still passing. ¯\_(ツ)_/¯
8aa9a0b
to
38a750a
Compare
8ffff9d
to
01b0ce3
Compare
01b0ce3
to
06988c2
Compare
This PR also updates react-native version of E2E app from 0.72 to 0.73. Unfortunately, this causes the E2E iOS app to not build with macos-12 runner with the following error:
Using macos-13 runner fixes this issue, but proceeds to break UI tests altogether with the following error:
E2E tests on both Android and iOS pass locally. But, it will take more investigation to fix iOS E2E tests. Since we will have to move the E2E tests to CircleCI eventually, I've decided to disable E2E tests in GHA and we will eventually add E2E tests back (on CircleCI). @mikenachbaur-okta |
@@ -49,133 +49,3 @@ jobs: | |||
-scheme "ReactNativeOktaSdkBridge" \ | |||
-destination "platform=iOS Simulator,OS=latest,name=iPhone 14" \ | |||
clean test | xcpretty | |||
iOSUITests: |
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.
Can you comment this out, instead of deleting it? This way it will help us to reenable it at a later date, if needed.
06988c2
to
15c439d
Compare
No description provided.