Skip to content

Conversation

markerikson
Copy link
Collaborator

This is very experimental stuff atm - I'm just pushing so the code is visible and I get some CI runs.

The idea here is to give the listener middleware additional capabilities that better match the feature set of sagas: cancelation of previous tasks for a a given listener, and the ability to launch child tasks. This would enable a large variety of use cases, like implementing the equivalent of takeLatest(), fork(), etc.

For reference, I asked about how people use those APIs here: https://twitter.com/acemarke/status/1465017211265994757

I've copied in the source code from https://github.com/ethossoftworks/job-ts , which is a very neat little lib that has a Job class that already implements parent/child hierarchical tasking and cancelation.

The first step was wrapping the call to the listener function in a new Job instance. This gives us a JobHandle that we can pass in as part of the listenerApi. Additionally, the Job library uses https://github.com/ethossoftworks/outcome-ts as a functional-Option-style handler for success/error results. I used that to consolidate the error handling for listeners, although I did drop the "sync/async" aspect to the error reporting.

Conceptually, I then want the condition/take functions to throw JobCancelationErrors if the current listener is canceled, and that's where my brain halted for the night. I was starting to try to restructure createTakePattern, but am realizing that A) the parent job doesn't give me a notice when it's canceled, and B) the call to addListener() wouldn't ever give me access to the nested listenerApi.job because the point is that cancelation happens before its listener ever runs.

So, a couple thoughts:

  • I could try adding an AbortController to the Job class so that you could listen for that
  • I could try rewriting take/condition to be based around the Job.pause() function instead

The latter approach is appealing, because there's already a Job.delay() function that has a timeout, it just doesn't return anything. It would be trivial to do the same thing but with a return value.

I really feel like I'm onto something here, but it's late and the pieces just aren't quite coming together in my head.

@markerikson
Copy link
Collaborator Author

/cc @FaberVitale fyi if you're interested

@netlify
Copy link

netlify bot commented Dec 2, 2021

✔️ Deploy Preview for redux-starter-kit-docs ready!

🔨 Explore the source changes: c4e1848

🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/61aae2c8a7d4b80007c231cc

😎 Browse the preview: https://deploy-preview-1792--redux-starter-kit-docs.netlify.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 2, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c4e1848:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
reduxjs/redux-toolkit Configuration
@examples-query-react/advanced Configuration

@github-actions
Copy link

github-actions bot commented Dec 2, 2021

size-limit report 📦

Path Size
1. entry point: @reduxjs/toolkit (cjs.production.min.js) 12.23 KB (0%)
1. entry point: @reduxjs/toolkit (esm.js) 10.22 KB (0%)
1. entry point: @reduxjs/toolkit/query (cjs.production.min.js) 20.35 KB (0%)
1. entry point: @reduxjs/toolkit/query (esm.js) 17.37 KB (0%)
1. entry point: @reduxjs/toolkit/query/react (cjs.production.min.js) 22.22 KB (0%)
1. entry point: @reduxjs/toolkit/query/react (esm.js) 19.82 KB (0%)
2. entry point: @reduxjs/toolkit (without dependencies) (cjs.production.min.js) 5.56 KB (0%)
2. entry point: @reduxjs/toolkit (without dependencies) (esm.js) 5.53 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (cjs.production.min.js) 9.2 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (esm.js) 9.58 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (cjs.production.min.js) 2.37 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (esm.js) 2.21 KB (0%)
3. createSlice (esm.js) 5.16 KB (0%)
3. createEntityAdapter (esm.js) 5.83 KB (0%)
3. configureStore (esm.js) 5.83 KB (0%)
3. createApi (esm.js) 15.66 KB (0%)
3. createApi (react) (esm.js) 18.05 KB (0%)
3. fetchBaseQuery (esm.js) 10.93 KB (0%)
3. setupListeners (esm.js) 9.8 KB (0%)
3. ApiProvider (esm.js) 16.99 KB (0%)

@FaberVitale
Copy link
Contributor

#1799

@markerikson
Copy link
Collaborator Author

Update: As of merging #1799 , we now have take and condition functions that appear to correctly re-throw JobCancellationException errors when the containing listener function gets canceled. That should enable the equivalent of takeLatest, by calling listenerApi.cancelPrevious() at the start of a listener. (This does assume that the previous instances are paused waiting for take/condition or the listenerApi.job.* methods that pause).

It's late and I want to do some more polishing before I publish a 0.4.0 (including updating the README), but this is looking very encouraging so far.

@markerikson markerikson force-pushed the feature/middleware-jobs branch from d267ac1 to 01e2464 Compare December 4, 2021 03:31
@markerikson markerikson force-pushed the feature/middleware-jobs branch from 01e2464 to c4e1848 Compare December 4, 2021 03:38
@markerikson
Copy link
Collaborator Author

markerikson commented Dec 4, 2021

Just added a test file demonstrating equivalence with multiple Redux-Saga operators:

 PASS  src/tests/effectScenarios.test.ts
  Saga-style Effects Scenarios
    √ throttle (51 ms)
    √ debounce / takeLatest (60 ms)
    √ takeEvery (24 ms)
    √ takeLeading (106 ms)
    √ fork + join (22 ms)
    √ fork + cancel (33 ms)
    √ canceled (69 ms)

I think that's sufficient evidence that this branch is worth merging and releasing.

@markerikson markerikson changed the title [DRAFT] Try adding a "jobs" system to the listener middleware Add a "jobs" system to the listener middleware Dec 4, 2021
@markerikson markerikson marked this pull request as ready for review December 4, 2021 03:40
@markerikson markerikson merged commit 18cf0cb into master Dec 4, 2021
@markerikson markerikson deleted the feature/middleware-jobs branch December 4, 2021 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants