Giter Club home page Giter Club logo

Comments (13)

nhusher avatar nhusher commented on May 28, 2024

It occurs to me that I should be using the built-in exception system:

function* createAudienceSaga (getState) {
  let action, cancelLastAudienceBuild = () => {};

  // When a new audience create action comes along, cancel the previous one
  // by resolving the currently-active promise. We're using a promise here
  // so that `race(...)` calls can be our mechanism for determining cancelation
  /*eslint no-cond-assign:0 */
  while (action = yield take(CREATE_AUDIENCE)) {
    cancelLastAudienceBuild(new TransactionCancelledError());

    let transaction = new Promise((_, reject) => cancelLastAudienceBuild = reject);

    yield fork(buildAudience, transaction, action.payload);
  }
}

// Create a new audience
function* buildAudienceFromPlaceId(tx, params) {
  try {
    let [place] = yield [call(api.places.get, id), tx];
    yield put(showPlace(place));

    const [ audienceId ] = [
      call(createAudienceWithDefinition, {
        definition: params.definition,
        geography: [makePlaceGeography(place)],
        product_id: params.product
      }), tx ];

    const [ audience ] = yield [call(api.audience.get, audienceId), tx ];

    yield put(showAudience(audience));    
  } catch (e) {
    if (e instanceof TransactionCancelledError) {
      console.info("Transaction cancelled", id, product);
      return;
    } else {
      throw e;
    }
  }
}

This resolves the issue to my satisfaction.

from redux-saga.

yelouafi avatar yelouafi commented on May 28, 2024

In your 2nd solution, this code

let [place] = yield [call(api.places.get, id), tx];

If call(api.places.get, id) resolves first, it'll have to wait also for tx to complete (which always rejects in your code)

The task cancellation would handle this case in a more concise way, until #26 is merged, I'd also use the builtin exception system, but would opt instead for a more declarative way

// a higher order function
function buildTx(cancelEffect) {
  // returns a reusable guarded call subroutine
  // will throw if cancelEffect completes before call(fn, ...args)
  return function* guardedCall(fn, ...args) {
    const {result} = yield race({
      result: call(fn, ...args),
      cancelEffect
    })

    if(result)
       return result
    throw new TransactionCancelledError()
  }
}


function* createAudienceSaga (getState) {
  let action
  const nextCreateAudience = take(CREATE_AUDIENCE)

  /*eslint no-cond-assign:0 */
  while (action = yield nextCreateAudience ) {
    yield fork(
        buildAudience, 
        nextCreateAudience,
        action.payload);
  }
}

// Create a new audience
function* buildAudienceFromPlaceId(cancelEffect, params) {
  const tx = buildTx(cancelEffect)
  try {
    let place = yield call(tx, api.places.get, id);
    yield put(showPlace(place));

    const audienceId = yield
      call(tx, createAudienceWithDefinition, {
        definition: params.definition,
        geography: [makePlaceGeography(place)],
        product_id: params.product
      });

    const audience = yield call(tx, api.audience.get, audienceId);

    yield put(showAudience(audience));    
  } catch (e) {
    if (e instanceof TransactionCancelledError) {
      console.info("Transaction cancelled", id, product);
      return;
    } else {
      throw e;
    }
  }
}

from redux-saga.

nhusher avatar nhusher commented on May 28, 2024

Thanks! I'll give this a look over and attempt to grok it.

from redux-saga.

nhusher avatar nhusher commented on May 28, 2024

I ended up taking your higher-order function and making it lower-order. This makes it easier to test:

export function* guard(tx, ...args) {
  const { result } = yield race({
    result: call(...args),
    tx
  });

  if(result) {
    return result;
  } else {
    throw new TransactionCancelledError();
  }
}
// Create a new audience from a place and product id
export function* buildAudienceFromPlaceId(tx, id, product) {
  try {
    // Step 1: retrieve the place that the user wants to build the audience in
    let place = yield call(guard, tx, api.places.get, id);
    // ...

It obscures intent somewhat because the actual function being called is deep in the arguments list, but it makes the test very straightforward:

import { guard } from 'wherever';

let tx = new Promise(() => {}),
    iterator = buildAudienceFromPlaceId(tx);

assert.deepEqual(
    iterator.next().value,
    call(guard, tx, api.places.get, placeId));

from redux-saga.

yelouafi avatar yelouafi commented on May 28, 2024

I see, you can also define a higher order effect creator

export function* guard(tx, ...args) {...}

const guardedCall = (guard, tx) => call(guard, tx, api.places.get, id);

// Create a new audience from a place and product id
export function* buildAudienceFromPlaceId(tx, id, product) {
  const callWithTx= guardedCall(guard, tx)
  try {
    // Step 1: retrieve the place that the user wants to build the audience in
    let place = yield callWithTx(api.places.get, id);
    // ...

from redux-saga.

nhusher avatar nhusher commented on May 28, 2024

I had also considered (but have not tested) the following because it uses private values that might break:

// A saga that waits for CREATE_AUDIENCE actions and uses the data to construct and load a new audience:
export default function* createAudience (getState) {
  let action, lastAction;

  while (action = yield take(CREATE_AUDIENCE)) {
    if(lastAction && lastAction.isRunning()) {
      lastAction._generator.throw(new TransactionCancelledError());
    }
    lastAction = yield fork(buildAudience, action.payload);
  }
}

Not sure what the implications of that are.

from redux-saga.

nhusher avatar nhusher commented on May 28, 2024

I see, you can also define a higher order effect creator

Yeah. Again, it means peeling the function instance out of the iterator.next().value, which I'd rather avoid if possible. If I use a lower-order function, I can share the function more overtly between the application code and tests, at the cost of somewhat obscured intent. Either way has tradeoffs, and I might change my mind later.

from redux-saga.

yelouafi avatar yelouafi commented on May 28, 2024

I had also considered (but have not tested) the following because it uses private values that might break

I wouldn't recommend this. As internal representation may change (actually already changed in the current master branch).

from redux-saga.

yelouafi avatar yelouafi commented on May 28, 2024

Yeah. Again, it means peeling the function instance out of the iterator.next().value

Maybe I missed something, but is there any issue with the following

code

export function* guard(tx, ...args) {...}

export const guardedCall = (guard, tx) => call(guard, tx, api.places.get, id);

// Create a new audience from a place and product id
export function* buildAudienceFromPlaceId(tx, id, product) {
  const callWithTx= guardedCall(guard, tx)
  try {
    // Step 1: retrieve the place that the user wants to build the audience in
    let place = yield callWithTx(api.places.get, id);
    // ...

test

import { guard, guardedCall } from 'wherever';

let tx = new Promise(() => {}),
     callWithTx = guardedCall(guard, tx),
     iterator = buildAudienceFromPlaceId(tx);

assert.deepEqual(
    iterator.next().value,
    callWithTx(api.places.get, placeId));

from redux-saga.

nhusher avatar nhusher commented on May 28, 2024

The result of a higher-order function is always unique:

function add(a) { return function(b) { return a + b; } }
assert(add(5) === add(5)); // untrue, fails

This means that the call to guardedCall in the generator won't equal the equivalent call in the test.

from redux-saga.

yelouafi avatar yelouafi commented on May 28, 2024

The result of a higher-order function is always unique:

yes, but in our current case we're not comparing the resulting functions but the final value

function add(a) { return function(b) { return a + b; } }
assert(add(5)(3) === add(5)(3)); 

assert.deepEqual(
  guardedCall(guard, tx)(api.places.get, placeId),
  guardedCall(guard, tx)(api.places.get, placeId)
)

from redux-saga.

yelouafi avatar yelouafi commented on May 28, 2024

It's not explicit in the docs, but all effect creators like call are pure functions. for example a call returns an object {fn, args}, so as long you're providing the same inputs, which is the case above for all the 4 arguments, you can expect the same output (in the deepEqual sens)

from redux-saga.

nhusher avatar nhusher commented on May 28, 2024

Oh yeah. I knew that but wasn't really understanding what was going on. Concern retracted.

Thanks for all your help, by the way!

from redux-saga.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.