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

Returning Promise<undefined> from getter triggers update #28

Open
traed opened this issue May 12, 2023 · 2 comments
Open

Returning Promise<undefined> from getter triggers update #28

traed opened this issue May 12, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@traed
Copy link

traed commented May 12, 2023

Hi!

I don't known if the docs are wrong or if this is not working as intended, but when running this example:

const posts = asyncable(async ($path, $query) => {
    if ($path.toString() === '/posts') {
      const res = await fetch(`/posts?page=${$query.params.page || 1}`);
      return res.json();
    }
  },
  null, 
  [ path, query ]
);

... the store will always be updated. That is because an async function (the getter in this case) will always return a Promise. In this case either the return value of res.json() or a Promise<undefined>. If we instead do this:

const posts = asyncable(($path, $query) => {
    if ($path.toString() === '/posts') {
      return fetch(`/posts?page=${$query.params.page || 1}`).then(res => res.json());
    }
  },
  null, 
  [ path, query ]
);

... it will work as stated in the docs because now the return type is either a Promise or undefined (not Promise<undefined>).

IMO this shouldn't matter. If the getter returns either undefined or Promise<undefined> no update should be triggered.

@PaulMaly
Copy link
Member

Hi, I think you're right. Current docs has a mistake about this. Only explicit undefined will stop store update, but probably we can expand this behavior and cover it with a case of Promise<undefined> as well.

@PaulMaly PaulMaly added the enhancement New feature or request label May 23, 2023
@PaulMaly
Copy link
Member

PaulMaly commented May 23, 2023

Oh, I found that we already discussed this question with community and actually such behavior was implemented in 1.3.0, but already in 1.3.1 it was rolled back with comment:

If getter wants to keep the current value, it should return undefined in an obvious way.

So, probably, I need to think about it more carefully. I'm not sure I remember the reasons of that. For now, I plan to update README to comply with current behavior, while I thinking about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants