Page MenuHomePhabricator

[lib] Fix circular dependency
ClosedPublic

Authored by michal on May 8 2023, 10:32 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jan 2, 12:24 PM
Unknown Object (File)
Thu, Dec 19, 8:52 PM
Unknown Object (File)
Sat, Dec 14, 10:29 PM
Unknown Object (File)
Sat, Dec 14, 10:29 PM
Unknown Object (File)
Sat, Dec 14, 10:29 PM
Unknown Object (File)
Sat, Dec 14, 10:27 PM
Unknown Object (File)
Sat, Dec 14, 10:07 PM
Unknown Object (File)
Dec 1 2024, 9:18 AM
Subscribers

Details

Summary

Should fix ENG-3840 (landing not working). The issue was a circular dependecy after adding validators. Moves session related functions out of account-utils and into session-utils

Test Plan

Check if web and landing work.

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu requested changes to this revision.May 8 2023, 10:56 AM

The code looks good, but I actually pitched this idea to @ashoat initially and we talked about finding another solution. Thinking out lout right now, but wouldn't it be possible to decouple this cycle by moving defaultNotificationPayloadValidator out of account-types and possibly into validation-utils? I think this would also be better since there are no other validators in account-utils and account-utils should exclusively just be for types

This revision now requires changes to proceed.May 8 2023, 10:56 AM

My high-level view is that it would be better to keep account-utils.js as a "leaf", and just extract the one function that is preventing account-utils.js from being a leaf, than to extract all of the other stuff into a new leaf

Instead of extracting the regexes, instead move the session functions. Only invalidSessionRecovery had to be moved but I moved both of them to keep them together, I can move invalidSessionDowngrade back if that's prefered.

defaultNotificationPayloadValidator out of account-types and possibly into validation-utils

I would prefer to keep as much validators near their flow definitions so people don't change one without changing the other

michal retitled this revision from [lib] Extract regexes from account-utils to [lib] Fix circular dependency.May 9 2023, 2:22 AM
michal edited the summary of this revision. (Show Details)

lgtm thanks for fixing this!

Only invalidSessionRecovery had to be moved but I moved both of them to keep them together,

this makes sense

This revision is now accepted and ready to land.May 9 2023, 3:01 AM