audius-client icon indicating copy to clipboard operation
audius-client copied to clipboard

Add RTK listenerMiddleware

Open sliptype opened this issue 2 years ago • 5 comments

Description

An example of using rtk listenerMiddleware instead of sagas. Some nice benefits:

  • Simpler syntax, no generator functions
  • Not compiled to switch statements, debugging is easier

    While looking into this I realized that metro-react-native-babel-preset includes @babel/plugin-transform-async-to-generator so async functions get transformed to generators regardless. However, hermes supports async/await out of the box, so maybe it would be wise to remove this plugin (could have a perf benefit)

Just a draft for now, would love to hear people's thoughts!

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

sliptype avatar Feb 13 '23 16:02 sliptype

Preview this change https://demo.audius.co/sk-listener-middleware

audius-infra avatar Feb 13 '23 17:02 audius-infra

omg

piazzatron avatar Feb 13 '23 18:02 piazzatron

Hype!! Maybe worth setting this up on web as well? Have we thought at all about how shared/common listeners will work?

Yeah I def think worth it for web too. Shared listeners should be similar to how we do sagas currently, can be defined in common and then a root level file in web/mobile can call each listener file with the appropriate startListening

sliptype avatar Feb 13 '23 18:02 sliptype

I absolutely love these changes! so this is just RTK addition, not a RTK-Query thing yeah? if so id say lets def move forward with this generally. itll probably cover 90% of usecases and be much simpler to understand

Yes exactly! I'm honestly just a little hesitant to bifurcate the codebase further though lol

And yeah cancellation is one of the cases it doesn't support, but offline is one of the few/only places we need that

sliptype avatar Feb 13 '23 18:02 sliptype

I absolutely love these changes! so this is just RTK addition, not a RTK-Query thing yeah? if so id say lets def move forward with this generally. itll probably cover 90% of usecases and be much simpler to understand

Yes exactly! I'm honestly just a little hesitant to bifurcate the codebase further though lol

yeah i think we should figure out what we are using for fetch layer first, but this changes that calculus

dylanjeffers avatar Feb 13 '23 18:02 dylanjeffers