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

KO Reactor / KO Undo Manager compatibility (context is not a function) #60

Open
silviuburceadev opened this issue Jul 10, 2020 · 1 comment

Comments

@silviuburceadev
Copy link

I am not sure where this issue belongs/must be fixed, so I'm opening in both repositories, since @bago is KO Reactor contributor and KO Undo Manager author.

According to KO Reactor (link), it supports a context option, which is undocumented. Looking at its usages in assignWatcher function, context should be a function.

However, KO Undo Manager is building an empty object (link) as context and is passing it to KO Reactor (link).

This leads to a TypeError: context is not a function error.

At this point, there are 2 options:

  • KO Undo Manager passing an empty function instead of empty object, which is what KO Reactor expects
  • KO Reactor checking if context is a function before executing it, e.g. typeof context === 'function' && context(returnValue) instead of context(returnValue).

Additionally, it might help if KO Reactor's context option is documented to indicate how it should look like.

I am happy to contribute with a PR if needed, I'm just not sure what's the correct way to handle this. Both options would be backwards-compatible.

@bago
Copy link
Collaborator

bago commented Jul 10, 2020

I replied to the other issue: bago/knockout-undomanager#5

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

No branches or pull requests

2 participants