Paths

Table of Contentst

Differential D4319

[lib] Simplify conditional logic in `wrapActionPromise:loadingInfo`
ClosedPublic
Actions

Authored by atul on Jun 21 2022, 4:14 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sat, Jul 6, 10:56 AM

	Unknown Object (File)
	Sat, Jul 6, 4:21 AM

	Unknown Object (File)
	Sat, Jul 6, 4:21 AM

	Unknown Object (File)
	Sat, Jul 6, 4:21 AM

	Unknown Object (File)
	Sat, Jul 6, 4:21 AM

	Unknown Object (File)
	Sun, Jun 30, 6:06 PM

	Unknown Object (File)
	Sun, Jun 30, 5:33 PM

	Unknown Object (File)
	Sun, Jun 30, 1:35 AM

Subscribers

Details

Reviewers

tomek
• jacek
• abosh
ashoat

Commits

rCOMM94a630eee7c2: Revert "[services][identity] add un-signed SIWE message to WalletLoginRequest"
rCOMMe8adf23f4dc8: [lib] Simplify conditional logic in `wrapActionPromise:loadingInfo`

Summary

Noticed that some of the logic here could be simplified w/ optional chaining + nullish coalescing... so made those changes

Test Plan

Close reading + flow

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Jun 21 2022, 4:14 PM

Herald added subscribers: • adrian, ashoat. · View Herald TranscriptJun 21 2022, 4:14 PM

atul added a reviewer: ashoat.Jun 21 2022, 4:15 PM

Harbormaster completed remote builds in B9861: Diff 13640.Jun 21 2022, 4:19 PM

atul requested review of this revision.Jun 21 2022, 4:19 PM

Looks good to me. Nice catch!

This revision is now accepted and ready to land.Jun 22 2022, 7:34 AM

atul added 1 blocking reviewer(s): tomek.Jun 22 2022, 9:22 AM

This revision now requires review to proceed.Jun 22 2022, 9:22 AM

tomek requested changes to this revision.Jun 22 2022, 9:35 AM

tomek added inline comments.

lib/utils/action-utils.js
64 ↗	(On Diff #13640)	This changes the behavior when `loadingOptions` is non-null and `customKeyName` is an empty string `''`. In the previous code, the result would be `null`. In the new code, the result is `''`. The solution to keep the behavior consistent is to use `\|\|` instead of `??`.

This revision now requires changes to proceed.Jun 22 2022, 9:35 AM

atul added inline comments.Jun 22 2022, 11:05 AM

lib/utils/action-utils.js
64 ↗	(On Diff #13640)	Ah thanks for catching that subtle bug. Will update this diff to use `\|\|` instead of `??`

address feedback from @palys-swm

atul attached a referenced file: F81139: dd38d4.png. (Show Details)Jun 22 2022, 11:09 AM

Harbormaster completed remote builds in B9887: Diff 13678.Jun 22 2022, 11:14 AM

ashoat accepted this revision.Jun 22 2022, 11:46 AM

tomek accepted this revision.Jun 23 2022, 2:09 AM

This revision is now accepted and ready to land.Jun 23 2022, 2:09 AM

rebase after cherrypicking and before landing

Harbormaster completed remote builds in B9923: Diff 13717.Jun 23 2022, 9:58 AM

Closed by commit rCOMMe8adf23f4dc8: [lib] Simplify conditional logic in `wrapActionPromise:loadingInfo` (authored by atul). · Explain WhyJun 23 2022, 10:20 AM

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMe8adf23f4dc8: [lib] Simplify conditional logic in `wrapActionPromise:loadingInfo`.

atul added a commit: rCOMM94a630eee7c2: Revert "[services][identity] add un-signed SIWE message to WalletLoginRequest".Jun 23 2022, 10:52 AM

Revision Contents
Changeset List

Path

Size

lib/

utils/

action-utils.js

9 lines

Diff 13720

lib/utils/action-utils.js

Loading...