[No QA] Fix all optional chains and nullish coalescing operator + update style guide #5988
Conversation
MelvinBot
requested review from
aldo-expensify
and removed request for
a team
STYLE.md
Outdated
| @@ -175,6 +175,23 @@ Empty functions (noop) should be declare as arrow functions with no whitespace i | |||
| } | |||
| ``` | |||
|
|
|||
| ## Accessing Object Properties and Default Values | |||
|
|
|||
| Use `lodashGet()` to safely access object properties and `||` to short circuit null or undefined values that are not guaranteed to exist in a consistent way throughout the codebase. In the rare case that you want to consider a falsy value as usable and the `||` operator prevents this then be explicit about this in your code and check for the type using an underscore method e.g. (`_.isBoolean(value)` or `_.isEqual(0)`). | |||
There was a problem hiding this comment.
Minimal detail... I think you are not supposed to have parenthesis after "e.g.", or another option could be:
...the type using an underscore method (e.g. _.isBoolean(value) or _.isEqual(0))
|
Updated |
I just read that we are deprecating lodashGet, are you planning to replaced the |
|
Ah no, I'm just changing the readme here. I have another PR that will accomplish this. |
| @@ -292,7 +309,8 @@ So, if a new language feature isn't something we have agreed to support it's off | |||
| Here are a couple of things we would ask that you *avoid* to help maintain consistency in our codebase: | |||
|
|
|||
| - **Async/Await** - Use the native `Promise` instead | |||
| - **Optional Chaining** - Use `lodash/get` to fetch a nested value instead | |||
| - **Optional Chaining** - Use `_.get()` to fetch a nested value instead | |||
There was a problem hiding this comment.
Is _ underscore? or lodash? maybe we should clarify since we have both libraries or... we asume _ is underscore because that is how we normally import underscore?
There was a problem hiding this comment.
It's underscore since whenever we use lodash we prefix with lodash*
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by @marcaaron in version: 1.1.8-10 🚀
|
roryabraham
changed the title
|
🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀
|
Details
This banishes optional chains and nullish coalescing operator from the codebase (for now). We have not explicitly said not to use this so I've added it to the style guide with alternatives that should be used instead. It is far more common to use
lodashGetfor property access and a||short circuit to default to a value whereundefinedornullis found.Fixed Issues
$ #5985
$ #5986
Tests / QA Steps
All of these changes should be syntax and shouldn't have any bearing on the app behavior.
Therefore, we just need to run regular regressions.
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android