Page MenuHomePhabricator

[lib] Fix invite links validation
ClosedPublic

Authored by inka on May 23 2024, 1:00 AM.
Tags
None
Referenced Files
F2187881: D12199.id40571.diff
Thu, Jul 4, 7:24 AM
Unknown Object (File)
Sat, Jun 29, 2:56 AM
Unknown Object (File)
Tue, Jun 25, 1:09 AM
Unknown Object (File)
Mon, Jun 24, 12:18 AM
Unknown Object (File)
Sun, Jun 23, 11:25 PM
Unknown Object (File)
Wed, Jun 19, 11:07 PM
Unknown Object (File)
Mon, Jun 17, 5:31 AM
Unknown Object (File)
Mon, Jun 17, 3:52 AM
Subscribers
None

Details

Summary

issue: ENG-8170
Invite links were failing validation, because union was handled incorrectly

This addresses the issue, but I still have to review other tcomb types ENG-8179

Test Plan

Tested that invite links work correclty now

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

lib/utils/user-info-extraction-utils.js
53 ↗(On Diff #40571)

Every use of any and its equivalents should be explicitly justified.

What else did you try here? Why is this absolutely necessary?

lib/utils/user-info-extraction-utils.js
31 ↗(On Diff #40571)

* is basically any and has been deprecated in Flow for a long time. This should never have been introduced in 2024. Please replace it

inka requested review of this revision.May 23 2024, 1:17 AM

Update types, fix maybe type

lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40578)

The any casts are now for the same reason as in D7488

Cannot instantiate TType because I is not a valid argument of TStructProps

Using the same approach as there

61 ↗(On Diff #40578)

Fixing maybe type as well, because otherwise update_user_avatar fails

ashoat added inline comments.
lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40578)

If you have a strong reason for using an any, you must still always re-cast it

60 ↗(On Diff #40578)

This one is safe because it gets immediately re-cast to TType<T> upon being returned

61 ↗(On Diff #40578)

Nice catch!

This revision is now accepted and ready to land.May 23 2024, 6:27 AM
lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40578)

I'm having trouble re-casting it. The correct type is TInterface<T>, but then I again get

Cannot instantiate TType because I is not a valid argument of TStructProps

In D7488 we had an input field which we would use to hack this problem.
I can cast this to TInterface<Object>... which is true, since TInterface<+T> props is of type TStructProps<T> = $ObjMap<T, TypeToValidator>. So as far as I understand, that means that T has to be an Object (from definition of $ObjMap)

lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40580)

Object is basically any as well. Can you use eg. { ... }?

Fix import

lib/utils/user-info-extraction-utils.js
55 ↗(On Diff #40580)

@ashoat and I tried changing this, but couldn't come up with anything that would work. We decided to leave the Object

This revision was automatically updated to reflect the committed changes.