Skip to content
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

DEV - Throws warning when starting desktop #40202

Closed
1 of 6 tasks
m-natarajan opened this issue Apr 13, 2024 · 34 comments
Closed
1 of 6 tasks

DEV - Throws warning when starting desktop #40202

m-natarajan opened this issue Apr 13, 2024 · 34 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Needs Reproduction Reproducible steps needed Weekly KSv2

Comments

@m-natarajan
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: Dev
Reproducible in staging?: dev
Reproducible in production?: dev
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @shubham1206agra
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1712819917386169

Action Performed:

  1. Open desktop app in dev environment

Expected Result:

Opens without any issues

Actual Result:

Error message is thrown

Onyx DevTools - Error: ReferenceError: window is not defined
[Electron] at DevTools.connectViaExtension (webpack://new.expensify/./node_modules/react-native-onyx/dist/DevTools.js?:14:35)
[Electron] at new DevTools (webpack://new.expensify/./node_modules/react-native-onyx/dist/DevTools.js?:6:31)
[Electron] at eval (webpack://new.expensify/./node_modules/react-native-onyx/dist/DevTools.js?:69:22)
[Electron] at ./node_modules/react-native-onyx/dist/DevTools.js (/Users/prachiagrawal/code/App/desktop/dist/main.js:2717:1)
[Electron] at webpack_require (/Users/prachiagrawal/code/App/desktop/dist/main.js:5411:42)
[Electron] at eval (webpack://new.expensify/./node_modules/react-native-onyx/dist/Onyx.js?:37:36)
[Electron] at ./node_modules/react-native-onyx/dist/Onyx.js (/Users/prachiagrawal/code/App/desktop/dist/main.js:2739:1)
[Electron] at webpack_require (/Users/prachiagrawal/code/App/desktop/dist/main.js:5411:42)
[Electron] at eval (webpack://new.expensify/./node_modules/react-native-onyx/dist/index.js?:7:32)
[Electron] at ./node_modules/react-native-onyx/dist/index.js (/Users/prachiagrawal/code/App/desktop/dist/main.js:2816:1)

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

@m-natarajan m-natarajan added Daily KSv2 Needs Reproduction Reproducible steps needed Bug Something is broken. Auto assigns a BugZero manager. labels Apr 13, 2024
Copy link

melvin-bot bot commented Apr 13, 2024

Triggered auto assignment to @mallenexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@MelvinBot
Copy link

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

@melvin-bot melvin-bot bot added the Overdue label Apr 15, 2024
@mallenexpensify
Copy link
Contributor

mallenexpensify commented Apr 15, 2024

Checking on in #engineering-chat

@melvin-bot melvin-bot bot removed the Overdue label Apr 15, 2024
@mallenexpensify
Copy link
Contributor

@pac-guerreiro , 👀 plz, this might be related to

@pac-guerreiro
Copy link
Contributor

@mallenexpensify Yeah... this is one me 😅 I can take a look into it if you want!

@mallenexpensify
Copy link
Contributor

Thanks for the quick reply, assigned to you. cc @marcaaron and @AndrewGable since you were the reviewers on the PR. Not assigning to either of ya, do so if you want.

@pac-guerreiro
Copy link
Contributor

@m-natarajan

I'm unable to reproduce this. Does the app crash to you? Are you running the app through npm run desktop?

@mallenexpensify mallenexpensify added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Apr 17, 2024
@mallenexpensify
Copy link
Contributor

Threw retest-weekly on here too.

@melvin-bot melvin-bot bot added the Overdue label Apr 19, 2024
Copy link

melvin-bot bot commented Apr 22, 2024

@mallenexpensify, @pac-guerreiro Huh... This is 4 days overdue. Who can take care of this?

@pac-guerreiro
Copy link
Contributor

@m-natarajan

Can you re-test this? 😄

@melvin-bot melvin-bot bot removed the Overdue label Apr 23, 2024
@mallenexpensify
Copy link
Contributor

@pac-guerreiro , @m-natarajan is only responsible for creating issues, they don't monitor or (often) respond to pings once issues are created. Let's wait til the retest-weekly label triggers a retest then we'll see what next best steps are.

@shubham1206agra
Copy link
Contributor

@pac-guerreiro @mallenexpensify Still able to repro on latest main.

@pac-guerreiro
Copy link
Contributor

@mallenexpensify

Thanks for the heads up! I didn't know about this, sorry 😅

@pac-guerreiro
Copy link
Contributor

@shubham1206agra

I was able to reproduce this and I'll create a PR with a fix for it 😄

@pac-guerreiro
Copy link
Contributor

This issue was caused by this PR 😅

@pac-guerreiro
Copy link
Contributor

pac-guerreiro commented Apr 25, 2024

My fix got merged and react-native-onyx was bumped to v2.0.34. We'll have to wait until we bump react-native-onyx version in the app

Copy link

melvin-bot bot commented Apr 27, 2024

@mallenexpensify @pac-guerreiro this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Apr 27, 2024
@mallenexpensify
Copy link
Contributor

We'll have to wait until the we bump react-native-onyx version in the app

Thinking we're waiting on the above.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Apr 30, 2024
Copy link

melvin-bot bot commented May 3, 2024

@mallenexpensify, @pac-guerreiro Whoops! This issue is 2 days overdue. Let's get this updated quick!

@pac-guerreiro
Copy link
Contributor

We'll have to wait until the we bump react-native-onyx version in the app

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 7, 2024
Copy link

melvin-bot bot commented May 10, 2024

@mallenexpensify, @pac-guerreiro Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented May 14, 2024

@mallenexpensify, @pac-guerreiro Still overdue 6 days?! Let's take care of this!

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels May 15, 2024
@melvin-bot melvin-bot bot removed the Overdue label May 15, 2024
@mallenexpensify
Copy link
Contributor

Bumped to weekly, per the comment above

We'll have to wait until the we bump react-native-onyx version in the app

@mvtglobally
Copy link

QA team is not able to confirm if this issue is still reproducible during KI retests as it requires Development Desktop app. This issue will need to be retested internally

@pac-guerreiro pac-guerreiro mentioned this issue May 16, 2024
50 tasks
@pac-guerreiro
Copy link
Contributor

Once this PR is merged, this issue should be resolved!

@pac-guerreiro
Copy link
Contributor

@mvtglobally

I can retest it once the PR I mentioned above gets merged! 😄

@mallenexpensify mallenexpensify removed the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label May 19, 2024
@pac-guerreiro
Copy link
Contributor

@mvtglobally @mallenexpensify

PR is now merged and the issue seems to be resolved! 😄

@mallenexpensify
Copy link
Contributor

Thanks @pac-guerreiro , gonna close. Comment/reopen if you disagree or if the issue persists.

@pac-guerreiro
Copy link
Contributor

@mallenexpensify

Looks like the PR that fixed this got reverted in #42725 😅

I'll keep you posted on any developments about this!

Meanwhile, this issue should show up again

@pac-guerreiro
Copy link
Contributor

@mallenexpensify

New PR is open - #42772 😄

@pac-guerreiro
Copy link
Contributor

Still waiting on #42772 😄

1 similar comment
@pac-guerreiro
Copy link
Contributor

Still waiting on #42772 😄

@pac-guerreiro
Copy link
Contributor

@mallenexpensify #42772 is finally merged so the issue should be resolved now! 😄

@mallenexpensify
Copy link
Contributor

Thx @pac-guerreiro , comment if anyone's due payment, it doesn't appear so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Needs Reproduction Reproducible steps needed Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants