Skip to content

[HOLD for payment 2024-11-07] Unable to run storybook locally #51108

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

Closed
8 tasks
chiragsalian opened this issue Oct 18, 2024 · 38 comments
Closed
8 tasks

[HOLD for payment 2024-11-07] Unable to run storybook locally #51108

chiragsalian opened this issue Oct 18, 2024 · 38 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@chiragsalian
Copy link
Contributor

chiragsalian commented Oct 18, 2024

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:
Reproducible in staging?:
Reproducible in production?:
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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:
Slack conversation: https://expensify.slack.com/archives/C02NK2DQWUX/p1727459623902909

Action Performed:

npm run storybook

Expected Result:

storybook should have opened without any issues

Actual Result:

Bunch of errors locally
image

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

cc @ishpaul777

Issue OwnerCurrent Issue Owner: @trjExpensify
@chiragsalian chiragsalian added External Added to denote the issue can be worked on by a contributor Weekly KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 18, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 18, 2024
Copy link

melvin-bot bot commented Oct 18, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @ishpaul777 (External)

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Oct 18, 2024
Copy link

melvin-bot bot commented Oct 18, 2024

Triggered auto assignment to @trjExpensify (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.

@Anaslancer
Copy link
Contributor

Anaslancer commented Oct 19, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

Unable to run storybook locally

What is the root cause of that problem?

The “cannot access before initialization” error occurs when you try to use a variable before it has been initialized.

What changes do you think we should make in order to solve the problem?

We should declare and initialize the variable before we use it.
The solution is to change this part.

let Link, LinkingContext, NavigationContainer, ServerContainer, DarkTheme, DefaultTheme, ThemeProvider, useLinkBuilder, useLinkProps, useLinkTo, useScrollToTop;
Link = realReactNavigation.Link;
LinkingContext = realReactNavigation.LinkingContext;
NavigationContainer = realReactNavigation.NavigationContainer;
ServerContainer = realReactNavigation.ServerContainer;
DarkTheme = realReactNavigation.DarkTheme;
DefaultTheme = realReactNavigation.DefaultTheme;
ThemeProvider = realReactNavigation.ThemeProvider;
useLinkBuilder = realReactNavigation.useLinkBuilder;
useLinkProps = realReactNavigation.useLinkProps;
useLinkTo = realReactNavigation.useLinkTo;
useScrollToTop = realReactNavigation.useScrollToTop;

What alternative solutions did you explore? (Optional)

N/A

Contributor details

Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01aff093c9a804b145

Copy link

melvin-bot bot commented Oct 19, 2024

📣 @Anaslancer! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@Anaslancer
Copy link
Contributor

Screenshot_4

Copy link

melvin-bot bot commented Oct 22, 2024

@trjExpensify, @ishpaul777 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Oct 22, 2024
@ishpaul777
Copy link
Contributor

i'll review proposals soon

@melvin-bot melvin-bot bot removed the Overdue label Oct 22, 2024
@ishpaul777
Copy link
Contributor

ishpaul777 commented Oct 22, 2024

Thanks for your proposal @Anaslancer, But I dont think your proposal identifies the root cause of the problem correctly, As I understand, it seems to be case of circular depency in importing of module @react-navigation/native we are importing the module in its mock itself.

const realReactNavigation = isJestEnv ? jest.requireActual<typeof ReactNavigation>('@react-navigation/native') : (require('@react-navigation/native') as typeof ReactNavigation);

@Anaslancer
Copy link
Contributor

Anaslancer commented Oct 23, 2024

Thank you, @ishpaul777
We are using jest.requireActual in this file when isJestEnv is true.

const realReactNavigation = isJestEnv ? jest.requireActual<typeof ReactNavigation>('@react-navigation/native') : (require('@react-navigation/native') as typeof ReactNavigation);

jest.requireActual(moduleName)
Returns the actual module instead of a mock, bypassing all checks on whether the module should receive a mock implementation or not.

So at the moment there is no error, because realReactNavigation works like object.
But when isJestEnv is false, we are just using the declarations. So we are getting the above error I think.
@ishpaul777 Please let me know your idea. Thanks

@ishpaul777
Copy link
Contributor

ishpaul777 commented Oct 23, 2024

we need to mock the used componenets from realReactNavigation, i am not sure if this is a best way to do this, open to more ideas...

diff --git a/__mocks__/@react-navigation/native/index.ts b/__mocks__/@react-navigation/native/index.ts
index 5bcafdc1856..1a56174209e 100644
--- a/__mocks__/@react-navigation/native/index.ts
+++ b/__mocks__/@react-navigation/native/index.ts
@@ -16,9 +16,11 @@ const {triggerTransitionEnd, addListener} = isJestEnv
           addListener: () => {},
       };
 
+const realOrMockedUseNavigation = isJestEnv ? realReactNavigation.useNavigation : {};
+
 const useNavigation = () => ({
-    ...realReactNavigation.useNavigation,
-    navigate: jest.fn(),
+    ...realOrMockedUseNavigation,
+    navigate: isJestEnv ? jest.fn() : () => {},
     getState: () => ({
         routes: [],
     }),
@@ -30,17 +32,17 @@ type NativeNavigationMock = typeof ReactNavigation & {
 };
 
 export * from '@react-navigation/core';
-const Link = realReactNavigation.Link;
-const LinkingContext = realReactNavigation.LinkingContext;
-const NavigationContainer = realReactNavigation.NavigationContainer;
-const ServerContainer = realReactNavigation.ServerContainer;
-const DarkTheme = realReactNavigation.DarkTheme;
-const DefaultTheme = realReactNavigation.DefaultTheme;
-const ThemeProvider = realReactNavigation.ThemeProvider;
-const useLinkBuilder = realReactNavigation.useLinkBuilder;
-const useLinkProps = realReactNavigation.useLinkProps;
-const useLinkTo = realReactNavigation.useLinkTo;
-const useScrollToTop = realReactNavigation.useScrollToTop;
+const Link = isJestEnv ? realReactNavigation.Link : () => null;
+const LinkingContext = isJestEnv ? realReactNavigation.LinkingContext : () => null;
+const NavigationContainer = isJestEnv ? realReactNavigation.NavigationContainer : () => null;
+const ServerContainer = isJestEnv ? realReactNavigation.ServerContainer : () => null;
+const DarkTheme = isJestEnv ? realReactNavigation.DarkTheme : {};
+const DefaultTheme = isJestEnv ? realReactNavigation.DefaultTheme : {};
+const ThemeProvider = isJestEnv ? realReactNavigation.ThemeProvider : () => null;
+const useLinkBuilder = isJestEnv ? realReactNavigation.useLinkBuilder : () => () => '';
+const useLinkProps = isJestEnv ? realReactNavigation.useLinkProps : () => ({});

@Anaslancer
Copy link
Contributor

@ishpaul777 I think it's best solution and it's similar like my above proposal.

@ishpaul777
Copy link
Contributor

ishpaul777 commented Oct 24, 2024

ok lets go with this solution #51108 (comment), lets assign @Anaslancer for PR

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Oct 24, 2024

Triggered auto assignment to @yuwenmemon, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@trjExpensify
Copy link
Contributor

Not really sure where to put this, but I guess being dev related we can put it in #quality.

@chiragsalian
Copy link
Contributor Author

LGTM, feel free to create the PR @Anaslancer.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Oct 28, 2024
@trjExpensify
Copy link
Contributor

Yep, PR has merged. Once it goes to prod, the 7-day regression period will start. Then payment will be made 👍

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 31, 2024
@melvin-bot melvin-bot bot changed the title Unable to run storybook locally [HOLD for payment 2024-11-07] Unable to run storybook locally Oct 31, 2024
Copy link

melvin-bot bot commented Oct 31, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Oct 31, 2024
Copy link

melvin-bot bot commented Oct 31, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.55-10 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-07. 🎊

For reference, here are some details about the assignees on this issue:

  • @ishpaul777 requires payment (Needs manual offer from BZ)
  • @Anaslancer requires payment (Needs manual offer from BZ)

Copy link

melvin-bot bot commented Oct 31, 2024

@ishpaul777 @trjExpensify The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@trjExpensify
Copy link
Contributor

Checklist time!

@Anaslancer
Copy link
Contributor

@trjExpensify When can we get the offer?

@ishpaul777
Copy link
Contributor

ishpaul777 commented Nov 9, 2024

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production
  • 2b. Reported on staging (deploy blocker)
  • 2c. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other: Agency Member
  • [Contributor] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

    Link to comment: https://github.com/Expensify/App/pull/41962/files#r1835364153

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

    Link to discussion: Not required

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again. Not possible for QA to test storybook this as of now

@trjExpensify
Copy link
Contributor

Payment summary as follows:

@Anaslancer can you link your Upwork profile, please? @ishpaul777, offer sent.

@Anaslancer
Copy link
Contributor

Anaslancer commented Nov 12, 2024

Here is my link. Thank you, @trjExpensify
https://www.upwork.com/freelancers/~01aff093c9a804b145

@trjExpensify
Copy link
Contributor

Thanks, sent you an offer!

Separately, was this you testing in the #exfy-roadmap public room earlier today?

image

@Anaslancer
Copy link
Contributor

Yes, it was me and deleted all test message.

@Anaslancer
Copy link
Contributor

There is any way to test on public room?
I am using the staging database.

@Anaslancer
Copy link
Contributor

Thank you for your offer and accepted :)

@trjExpensify
Copy link
Contributor

Yes, it was me and deleted all test message.

Thanks for confirming! Please don't use any of the EXFY owned public rooms for testing. They include real customers and investors.

There is any way to test on public room?

You can create your own, but feel free to use this testing one as well: https://staging.new.expensify.com/r/2091104345528462

@trjExpensify
Copy link
Contributor

Thank you for your offer and accepted :)

Paid, @Anaslancer. @ishpaul777 waiting on you to accept your offer now as well.

@trjExpensify
Copy link
Contributor

@ishpaul777 bump!

@ishpaul777
Copy link
Contributor

ishpaul777 commented Nov 15, 2024

I just landed 🛬 in my home city was OOO past few days will be catching up on overdue things soon, Thanks for bump and sorry for delay

@ishpaul777
Copy link
Contributor

I have accepted the offfer 🙇

@trjExpensify
Copy link
Contributor

Paid, closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

5 participants