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

onSaved needs workaround to work #2

Open
w7tf opened this issue Oct 17, 2024 · 5 comments
Open

onSaved needs workaround to work #2

w7tf opened this issue Oct 17, 2024 · 5 comments

Comments

@w7tf
Copy link
Owner

w7tf commented Oct 17, 2024

Currently onSaved is done the following way:

onSaved: ({ saved }) => {
      const post = store$.posts
        .filter((post) => saved.id === post.id.get())[0]
        .get();
      post.updatedAt = saved.updatedAt; // no set to not retrigger any crud ops
      post.createdAt = saved.createdAt; // no set to not retrigger any crud ops
    },
@jmeistrich
Copy link

The observable should be updated automatically with the saved value from create and update, as long as those return with the updated values (which it seems like they do). So this shouldn't be necessary at all. Are they not updating automatically?

@w7tf
Copy link
Owner Author

w7tf commented Oct 17, 2024

The observable should be updated automatically with the saved value from create and update, as long as those return with the updated values (which it seems like they do). So this shouldn't be necessary at all. Are they not updating automatically?

They are updating automatically if as is set to object, it does not work if as is set to array

@jmeistrich
Copy link

Oh gotcha! Ok I've reproduced it in a test and will fix it.

For what it's worth I'd recommend probably not using as: 'array'. It's usually much easier to update items in an object or Map by id, whereas with an array you end up running find a lot.

@w7tf
Copy link
Owner Author

w7tf commented Oct 17, 2024

Oh gotcha! Ok I've reproduced it in a test and will fix it.

For what it's worth I'd recommend probably not using as: 'array'. It's usually much easier to update items in an object or Map by id, whereas with an array you end up running find a lot.

Yeah good input thanks! I forgot to think about O-complexity when I set it to use as array. Apart from the Object.values(obj) syntax when listing the whole state it makes much more sense to as object. Will refactor the store to use as object.

@jmeistrich
Copy link

Ok cool, sounds good! I have a fix coming in the next patch but I won't rush it since you don't need it urgently :)

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