Bump react-native-web to 0.15.7 react to 17.0.2 #3215

marcaaron · 2021-05-28T20:09:47Z

Details

Coming from #2939

I'm gonna recommend that we do not update to react-native-web 0.16 yet because there is a change to EventEmitter that is causing logs to stream in the console. Probably this will change when we update other dependencies that reference the more recent versions of react-native.

Fixed Issues (Snyk PR)

#2939

Tests

QA Steps

Test opening links and verify they open in new tabs on web
Test for general regressions in all platforms

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

roryabraham · 2021-05-28T20:42:19Z

I'm gonna recommend that we do not update to react-native-web 0.16 yet because there is a change to EventEmitter that is causing logs to stream in the console

👍

marcaaron · 2021-05-28T20:58:39Z

There is a bug on main preventing Android testing (reverted the code here so we could test), but this is working pretty good for me.

Julesssss · 2021-06-02T09:11:25Z

FYI, conflicts

marcaaron · 2021-06-08T00:58:32Z

Updated

Julesssss · 2021-06-08T15:30:10Z

Damn, sorry @marcaaron -- some recent merge has triggered further conflicts

marcaaron · 2021-06-08T17:01:03Z

Thanks, conflicts fixed

src/components/Tooltip/index.js

src/components/InvertedFlatList/index.js

Julesssss

Changes look good and it tested well. Added a few questions

marcaaron · 2021-06-08T23:55:46Z

Gonna run through platform tests again since we just updated 7 zillion other things.

marcaaron · 2021-06-09T00:29:28Z

Ok ready for review again

Julesssss · 2021-06-09T11:10:32Z

src/libs/Navigation/Navigation.js

-/**
- * Determines whether the drawer is currently open.
- *
- * @returns {Boolean}
- */
-function isDrawerOpen() {
-    return getIsDrawerOpenFromState(navigationRef.current.getRootState().routes[0].state);
-}
-
 export default {


Was this just removed for cleanup?

Julesssss

Looking good. Ran through all flow I could think of without running into any issues.

OSBotify · 2021-06-09T12:30:48Z

🚀 Deployed to staging in version: 1.0.65-10🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

isagoico · 2021-06-09T16:03:47Z

Links are opening in the same tab

Expected Result

Links should open in a different tab

Actual result

Links are opening in the same tab

Action Performed

Navigate to a conversation and send a link
Click on the link in the conv

Platform

Web ✔️
mWeb ✔️

Notes/images/video
(I'm on mobile atm and unable to upload the vid getting the video in a bit)

OSBotify · 2021-06-09T16:05:17Z

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

Identify the pull request that introduced this issue and revert it.
Find someone who can quickly fix the issue.
Fix the issue yourself.

marcaaron · 2021-06-09T16:33:58Z

Navigate to a conversation and send a link

Hmm.. I think this is possibly related to this PR -> #3066

And not this one... cc @roryabraham - also lmk if that's incorrect and I can look into this some more.

roryabraham · 2021-06-09T17:37:50Z

Hmm.. I think this is possibly related to this PR -> #3066

I don't think so ... I believe I checked regular links in the testing + QA steps of that PR. Also the _target wasn't changed, just certain URLs are given an ?authToken=xyz where they were not before.

marcaaron · 2021-06-09T17:38:57Z

Ok cool maybe it is related to react-native-web accessibility stuff. I'm unsure how those links are working.

Julesssss · 2021-06-09T17:40:08Z

If it helps, I'm pretty sure I didn't try opening links during testing.

marcaaron · 2021-06-09T17:48:02Z

Copied the markup from one of these links and got this

<a href="https://google.com" role="link" class="css-reset-4rbku5 css-cursor-18t94o4 css-text-901oao css-textHasAncestor-16my406" style="color: rgb(1, 133, 255); font-family: GTAmericaExp-Regular; font-size: 15px; line-height: 20px; text-decoration-color: rgb(1, 133, 255); text-decoration-line: underline;">https://google.com</a>

so seems like the target is not getting passed now, but unsure why.

marcaaron · 2021-06-09T17:53:10Z

Seems like we need to use hrefAttrs as per -> https://necolas.github.io/react-native-web/docs/accessibility/#links

Fix PR incoming.

stitesExpensify · 2021-06-10T18:14:58Z

It looks like this PR is responsible for #3512

It seems like the new react native web is eating shortcuts for some reason when focused on inputs. This happens when doing cmd+k for the sidebar, and pressing escape to remove the sidebar.

Julesssss · 2021-06-11T12:59:56Z

I had a quick look through the 0.16.0 changes, but couldn't find anything obvious that would break shortcut keys.

It's also odd that no one else has reported this issue (the release was back in April).

Julesssss · 2021-06-11T14:46:31Z

It looks to me like this react-native-web update is also responsible for this regression. Perhaps we should think about reverting for now?

Julesssss · 2021-06-11T15:01:48Z

I'm leaning towards reverting this PR as it introduced two regressions that block our N5 build and we're struggling to make progress on these fixes.

Does anyone disagree? It's a bit of a pain to revert, CP and review again but I can't see much progress being made here soon and we're hoping to release on Monday.

CC @trjExpensify

stitesExpensify · 2021-06-11T16:38:21Z

If we're planning on releasing Monday I agree, we should just revert. These are changes to 2 of our core components and if we already have 2+ regressions, I would be willing to bet there are more

marcaaron · 2021-06-11T16:53:55Z

If we want to revert this then we will need to also roll back #3486

Also heads up, if we wants this to happen before Monday please get someone else to help with the revert and testing as I can't manage this. We might also want to keep an eye on the react-native version bump made here if we are worried about

if we already have 2+ regressions, I would be willing to bet there are more

stitesExpensify · 2021-06-11T18:07:40Z

if we wants this to happen before Monday please get someone else to help with the revert and testing as I can't manage this

I also can't take care of this 😅 I'm working half days Monday/Tuesday

Julesssss · 2021-06-14T15:37:43Z

I was mistaken. This PR didn't introduce the issue -- it started occurring after main was merged into this PR, so here is the list of libraries that could have introduced the regression.

OSBotify · 2021-06-15T16:54:01Z

🚀 Deployed to production in version: 1.0.68-4🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

marcaaron added 3 commits May 28, 2021 07:35

remove openURLInNewTab

5ce0b06

Bump versions fix InvertedFlatlist listener issue

13cfac1

undo other dependency updates

4d7f9c2

marcaaron self-assigned this May 28, 2021

remove autoCorrect

30d66b7

marcaaron mentioned this pull request May 28, 2021

[Snyk] Upgrade react-native-web from 0.14.10 to 0.16.1 #2939

Closed

marcaaron marked this pull request as ready for review May 28, 2021 21:07

marcaaron requested a review from a team as a code owner May 28, 2021 21:07

MelvinBot requested review from Julesssss and removed request for a team May 28, 2021 21:07

marcaaron mentioned this pull request May 28, 2021

[Snyk] Upgrade react-native-web from 0.14.10 to 0.16.2 #3220

Merged

fix conflicts

9a0deec

marcaaron added 2 commits June 8, 2021 06:48

fix conflicts

822acf0

add e.preventDefault()

a4dbd43

Julesssss reviewed Jun 8, 2021

View reviewed changes

src/components/Tooltip/index.js Show resolved Hide resolved

Julesssss reviewed Jun 8, 2021

View reviewed changes

src/components/InvertedFlatList/index.js Show resolved Hide resolved

Julesssss reviewed Jun 8, 2021

View reviewed changes

marcaaron requested a review from roryabraham June 8, 2021 19:01

marcaaron added 2 commits June 8, 2021 13:51

fix conflicts

c0046a2

update podfile and package-lock.json

adfb815

remove getIsDrawerOpenFromState

e75416e

Julesssss reviewed Jun 9, 2021

View reviewed changes

Julesssss approved these changes Jun 9, 2021

View reviewed changes

Julesssss merged commit 84f0b3e into main Jun 9, 2021

Julesssss deleted the marcaaron-bumpReactNativeWeb branch June 9, 2021 11:48

OSBotify mentioned this pull request Jun 9, 2021

Deploy Checklist: Expensify.cash 2021-06-08 #3450

Closed

49 tasks

isagoico added the DeployBlockerCash This issue or pull request should block deployment label Jun 9, 2021

github-actions bot added Engineering Hourly KSv2 labels Jun 9, 2021

marcaaron mentioned this pull request Jun 9, 2021

Use hrefAttrs for links #3486

Merged

5 tasks

stitesExpensify mentioned this pull request Jun 10, 2021

CMD + K is not working when the focus is on compose box #3512

Closed

Julesssss mentioned this pull request Jun 11, 2021

[HOLD for payment 21 June] iOS - Attachment picker flickers and unable to select for choose document #3543

Closed

Jag96 mentioned this pull request Jun 11, 2021

[HOLD for RN-Modal to support RN 0.65] [Windows only] Cntrl + click doesn't open the link in a new tab for MenuItem component. #3455

Closed

Julesssss removed the DeployBlockerCash This issue or pull request should block deployment label Jun 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump react-native-web to 0.15.7 react to 17.0.2 #3215

Bump react-native-web to 0.15.7 react to 17.0.2 #3215

marcaaron commented May 28, 2021 •

edited

Loading

roryabraham commented May 28, 2021

marcaaron commented May 28, 2021

Julesssss commented Jun 2, 2021

marcaaron commented Jun 8, 2021

Julesssss commented Jun 8, 2021

marcaaron commented Jun 8, 2021

Julesssss left a comment

marcaaron commented Jun 8, 2021

marcaaron commented Jun 9, 2021

Julesssss Jun 9, 2021

marcaaron Jun 11, 2021

Julesssss left a comment

OSBotify commented Jun 9, 2021

isagoico commented Jun 9, 2021

OSBotify commented Jun 9, 2021

marcaaron commented Jun 9, 2021

roryabraham commented Jun 9, 2021

marcaaron commented Jun 9, 2021

Julesssss commented Jun 9, 2021

marcaaron commented Jun 9, 2021

marcaaron commented Jun 9, 2021

stitesExpensify commented Jun 10, 2021

Julesssss commented Jun 11, 2021

Julesssss commented Jun 11, 2021

Julesssss commented Jun 11, 2021 •

edited

Loading

stitesExpensify commented Jun 11, 2021

marcaaron commented Jun 11, 2021

stitesExpensify commented Jun 11, 2021

Julesssss commented Jun 14, 2021

OSBotify commented Jun 15, 2021

Bump react-native-web to 0.15.7 react to 17.0.2 #3215

Bump react-native-web to 0.15.7 react to 17.0.2 #3215

Conversation

marcaaron commented May 28, 2021 • edited Loading

Details

Fixed Issues (Snyk PR)

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

roryabraham commented May 28, 2021

marcaaron commented May 28, 2021

Julesssss commented Jun 2, 2021

marcaaron commented Jun 8, 2021

Julesssss commented Jun 8, 2021

marcaaron commented Jun 8, 2021

Julesssss left a comment

Choose a reason for hiding this comment

marcaaron commented Jun 8, 2021

marcaaron commented Jun 9, 2021

Julesssss Jun 9, 2021

Choose a reason for hiding this comment

marcaaron Jun 11, 2021

Choose a reason for hiding this comment

Julesssss left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 9, 2021

isagoico commented Jun 9, 2021

Links are opening in the same tab

Expected Result

Actual result

Action Performed

Platform

OSBotify commented Jun 9, 2021

marcaaron commented Jun 9, 2021

roryabraham commented Jun 9, 2021

marcaaron commented Jun 9, 2021

Julesssss commented Jun 9, 2021

marcaaron commented Jun 9, 2021

marcaaron commented Jun 9, 2021

stitesExpensify commented Jun 10, 2021

Julesssss commented Jun 11, 2021

Julesssss commented Jun 11, 2021

Julesssss commented Jun 11, 2021 • edited Loading

stitesExpensify commented Jun 11, 2021

marcaaron commented Jun 11, 2021

stitesExpensify commented Jun 11, 2021

Julesssss commented Jun 14, 2021

OSBotify commented Jun 15, 2021

marcaaron commented May 28, 2021 •

edited

Loading

Julesssss commented Jun 11, 2021 •

edited

Loading