Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to skip spread updates with special token #328

Open
zerobias opened this issue May 7, 2024 · 3 comments
Open

Allow to skip spread updates with special token #328

zerobias opened this issue May 7, 2024 · 3 comments
Labels
RFC Some new feature that should be discussed

Comments

@zerobias
Copy link
Member

zerobias commented May 7, 2024

One of key features of spread is an ability to skip updates for some targets, but it requires object mutation or weird conditionals (it becomes worse when amount of targets grows):

sample({
  clock: trigger,
  fn: upd => upd.both
    ? ({foo: 0, bar: 0})
    : ({foo: 0})
  target: spread({
    foo,
    bar,
  })
})

// or

sample({
  clock: trigger,
  fn(upd) {
    const result = {foo: 0}
    if (upd.both) {
      result.bar = 0
    }
    return result
  },
  target: spread({
    foo,
    bar,
  })
})

We can add special token SKIP which will mean that this unit will not be triggered:

import {spread, SKIP} from 'patronum'

// or maybe spread.SKIP

sample({
  clock: trigger,
  fn: upd =>({
    foo: 0,
    bar: upd.both ? 0 : SKIP
  }),
  target: spread({
    foo,
    bar,
  })
})
@zerobias zerobias added the enhancement Improvement in existing feature label May 7, 2024
@zerobias
Copy link
Member Author

zerobias commented May 7, 2024

After internal discussion we came to the conclusion that this would be an inconsistent solution - it misleads people into thinking that it is fn that does the filtering when it is spread that does it, it introduces new complex concepts to solve a very small problem.

This problem is easier to solve with native methods, although it may be a slightly more verbose solution

sample({
  clock: trigger,
  fn: (upd) => ({
    foo: 0,
    ...(upd.both ? {bar: 0} : null),
    ...(upd.baz ? {baz: 0} : null),
  }),
  target: spread({
    foo,
    bar,
    baz,
  })
})

@sergeysova sergeysova added RFC Some new feature that should be discussed and removed enhancement Improvement in existing feature labels May 7, 2024
@zerobias
Copy link
Member Author

zerobias commented May 7, 2024

But the underlying issue remains opened:

  1. A trigger from a single data source is required
  2. For multiple units at once
  3. With support for data transformation
  4. With the ability to skip updates of any of them by arbitrary criteria

We'll continue to think about this idea, maybe go back to the option suggested above, maybe solve it some other way

@AlexandrHoroshih
Copy link
Member

AlexandrHoroshih commented May 7, 2024

Actually, it looks like someting that prepend operator (proposed quite a while ago) should handle

sample({
 clock: trigger,
 target: spread({
    first: $first,
    second: prepend({
      filter: $allowed,
      fn: mapData
      target: $second
    }),
  }),
});

It does check all boxes:

  • Supports the ability to skip updates by arbitrary criteria
  • Supports data transformation
  • Can be applied to multiple units (one can combine it with another spread to make it multiple units at once)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Some new feature that should be discussed
Projects
None yet
Development

No branches or pull requests

3 participants