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

combine works strange with events #509

Closed
PFight opened this issue Aug 10, 2021 · 1 comment
Closed

combine works strange with events #509

PFight opened this issue Aug 10, 2021 · 1 comment
Assignees
Labels
core effector package RFC Request for Comments

Comments

@PFight
Copy link

PFight commented Aug 10, 2021

Proposal

When event passed to combine we should raise error, or process it some how.

Use case

I have wrote next code, and found, that it is not works:

const c1 = createStore(true);
const eventX = createEvent();
const combinedStore = combine([ c1, eventX], (info) => {
  console.info("combine reducer: " + info);
  return info[0] && info[1];
});
combinedStore.watch(combined => console.info("combined: " + combined));
eventX(true);

I expected, that it will works somehow like this (more precisely, I exepcted, that before first eventX call info will contain single element - c1 value):

const c1 = createStore(true);
const eventX = createEvent();
const combinedStore = combine([ c1, restore(eventX, null)], (info) => {
  console.info("combine reducer: " + info);
  return info[0] && info[1];
});
combinedStore.watch(combined => console.info("combined: " + combined));
eventX(true);

But instead, it works strange: event passed to reducer as is. I think, due to principle "Predictability and clarity of API" combine should process events some how correctly, or raise typescript error, or runtime error. Current behaviour is totally unexpected.

@PFight PFight added the RFC Request for Comments label Aug 10, 2021
@zerobias
Copy link
Member

Completely agree, will implement error raising in the next release.
Automatic restore will violate type of event, so this is not an option, unfortunately:

const evt: Event<number>
combine(evt, num => num.toFixed())
// Error: null is not an object

@zerobias zerobias added the core effector package label Aug 14, 2021
@zerobias zerobias self-assigned this Aug 14, 2021
zerobias added a commit that referenced this issue Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core effector package RFC Request for Comments
Projects
None yet
Development

No branches or pull requests

2 participants