Update Flow to 0.274.2 and migrate to match structures #3586
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
reosarevok
force-pushed
the
flow-274
branch
4 times, most recently
from
bb0a88b to
5aaed18
Compare
No errors https://github.com/facebook/flow/releases/tag/v0.274.1 Allows for the new match pattern: https://flow.org/en/docs/match/ AFAICT this can replace our exhaustive(action); switches, might look into it in a subsequent commit.
Just fixes some bugs in the match implementation https://github.com/facebook/flow/releases/tag/v0.274.2
This uses the new match structure to replace all the exhaustive switches. This allows us to drop the use of exhaustive() itself. This also drops all the error throwing in default cases which should never be reached if our typing is correct anyway. See: https://flow.org/en/docs/match/ https://flow.org/en/docs/match/migration/
This migrates most Flow-covered switch structures, since match is nicer for exhaustiveness (I fixed a couple of issues with unused actions and missing strings here thanks to that).
One match seems a lot clearer than the previous system.
This drops the always hacky (and defined as "a Flow landmine") chooseLayoutComponent picker and uses Flow's match feature to pick a layout based on the actual entity being displayed.
We had a separate type RecordingWithArtistCreditT because TO_JSON didn't always come with an artist credit. AFAICT, the same applied to RGs and releases, so in theory we should probably also have had ReleaseWithAC and RGWithAC . This leads to a huge pain maintaining Flow types. AFAICT, there is no reason why we shouldn't just always return an AC for entities that can have it - just an empty one, which is valid in Flow. This adds a new function nonEmptyArtistCredit() to use in places where we care about the AC being empty (in order to fall back to another one for example).
This is used in a ton of places, and it's mirroring a specific kind of response from the Perl side. It seems sensible to have it as a default type like the TO_JSON ones.
This was using EditorT, but what it actually gets is AccountLayoutUserT (editors explicitly go through serialize_user).
reosarevok
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No new errors.
0.274.1 introduces the new match feature (0.274.2 fixes some bugs in said feature).
This is a super neat option that allows to replace
switchstructures with something that Flow understands a lot better (and that checks forexhaustiveness by default, meaning no need to callexhaustiveexplicitly anymore. I converted most of ourswitchto usematchnow. Some are better to leave as-is for now, either because we very much don't want exhaustiveness and it would not help with clarity, or because they usereduxand new mock actions are introduced on runtime that we don't want to bother with just yet (since we'll migrate them eventually).Additionally, this gets rid of the hacky
chooseLayoutComponentpicker and replaces it with a much more sensible and type-safeLayoutComponentusingmatch. In order to make Flow entirely happy with it I got rid of theRecordingWithArtistCreditTdistinction (now all entities with artist credits always are sent as JSON with an artist credit, an empty one is added if they lacked one). I changed a bunch of places to check whether the AC is empty rather than whether it exists to go along with this change. I also fixed the types forSubscribers(which claimed to be getting anEditorTbut was actually getting anAccountLayoutUserT).Finally, I removed a bunch of Flow ignores that no longer trigger Flow issues after some bug fix or another in the previous updates.