Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.48k stars 2.84k forks source link

[$250] Console error cleanup. #51099

Open mallenexpensify opened 1 day ago

mallenexpensify commented 1 day ago

Apologies for janky formatting below. The post in #expensify-open-source is easier to read

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack

:broom: Some clean-up report on console errors / warnings as mentioned by @carlos here, quoting:

The idea is that we never have these errors and warnings, but we haven't been as diligent with this as should've been, so we've accumulated some warnings and errors, and we should fix them (I saw some warnings just now on another PR review).

The below errors / warnings were found on local dev, latest main w/ StrictMode enabled :downvote: Note: Keep in mind that these were discovered during the main login flow, there are probably more if we dive deep into every flow (including Accounting integrations / Expensify Card, etc).

:exclamation: Errors (Web)

  1. Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Wrap which is inside StrictMode. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://reactjs.org/link/strict-mode-find-node[](https://reactjs.org/link/strict-mode-find-node%60) - Origin file / line: react-dom.development.js:86
  2. Warning: Using UNSAFE_componentWillReceiveProps in strict mode is not recommended and may indicate bugs in your code. See https://reactjs.org/link/unsafe-component-lifecycles for details. - Origin file / line: react-dom.development.js:86

:warning: Warnings (Web & Native)

  1. Animated: 'useNativeDriver' is not supported because the native animated module is missing. Falling back to JS-based animation. To resolve this, add 'RCTAnimation' module to this app, or remove 'useNativeDriver'. Make sure to run 'bundle exec pod install' first. Read more about autolinking: - Origin file / line: NativeAnimatedHelper.js:441
  2. accessibilityLabel is deprecated. Use aria-label. - Origin file / line: index.js:28
  3. "shadow*" style props are deprecated. Use "boxShadow". - Origin file / line: vendors-node_modules_react-navigation_stack_lib_module_navigators_createStackNavigator_js-nod-760534-30b975cdb07db961ae15.bundle.js:403
  4. props.pointerEvents is deprecated. Use style.pointerEvents - Origin file / line: index.js:28
  5. returnKeyType is deprecated. Use enterKeyHint. - Origin file / line: index.js:28

Android / iOS Native

  1. findHostinstance_DEPRECATED is deprecated in StrictMode. findHostinstance_DEPRECATED was passed an instance of AnimatedComponent(Textinput) which is inside StrictMode. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://react.dev/link/strict-mode-find-node - Origin file / line: fabricUtils.ts:33
  2. Require cycle: src/CONST.ts -> src/libs/models/ BankAccount.ts -> src/CONST.ts Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle. - Origin file / line: BankAccount.ts:32
  3. More Require cycle warnings in multiple files (48 instances).

Not sure what's the proto regarding who should fix and when should these be fixed but I guess there are 2 main options: :one: Somebody opens one PR where all of these are addressed & fixed. :two: Willing C+ which currently have opened PRs by contributors pick 1-2 errors / warnings to fix and merge with already opened PRs. ((3 votes for 1, none for 2))

cc @ikevin127 @cead22 @fabioh8010

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021847371950143140066
  • Upwork Job ID: 1847371950143140066
  • Last Price Increase: 2024-10-18
Issue OwnerCurrent Issue Owner: @eVoloshchak
cead22 commented 1 day ago

I say we make this an external task like we do for normal bugs

fabioh8010 commented 1 day ago

@cead22 Do you mind if we pass this to our expert agency like we discussed here?

I already passed context to @kubabutkiewicz to work on this, and we believe this task needs more careful planning and coordination to do all these fixings.

cead22 commented 1 day ago

That works for me

mallenexpensify commented 1 day ago

@fabioh8010 , want me to assigne to @kubabutkiewicz? I'll make it a weekly too. I'm assuming we'll want C+ to review any PRs, comment if anyone disagrees.

fabioh8010 commented 1 day ago

Yes @mallenexpensify Please re-assign to @kubabutkiewicz ! Thanks

mallenexpensify commented 1 day ago

@kubabutkiewicz can you comment please so I can assign? Thx

kubabutkiewicz commented 1 day ago

Hello, Im Jakub from Callstack and would like to help with this issue

melvin-bot[bot] commented 1 day ago

Job added to Upwork: https://www.upwork.com/jobs/~021847371950143140066

melvin-bot[bot] commented 1 day ago

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

ikevin127 commented 1 day ago

[!tip] The warnings / errors listed by me in the Slack 🧵 are just the tip of the 🧊berg, the ones everyone can see as soon as the app is opened but there are many others that show up only when specific pages are opened (lots of lists with duplicated keys) and also many other that are platform specific, for example showing up on Android: Native or iOS: Native, some are Desktop specific (electron).