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

parket recompute the views too many times #8

Closed
tinchoz49 opened this issue Feb 1, 2018 · 5 comments
Closed

parket recompute the views too many times #8

tinchoz49 opened this issue Feb 1, 2018 · 5 comments

Comments

@tinchoz49
Copy link

tinchoz49 commented Feb 1, 2018

Hi @ForsakenHarmony! how are you?

I've been testing the library and I saw that for only one mutation a view is recomputed four times. This can be a performance issue if you are working with expensive computations like filtering arrays.

https://repl.it/@MartinAcosta/views-parket

import model from 'parket';

const Person = model('Person', { // name is used internally for serialization
  initial: () => ({
    firstname: null,
    lastname: null
  }),
  actions: state => ({
    setFirstName (first) {
      state.firstname = first; // no set state, no returns to merge, it's reactive™
    },
    setLastName (last) {
      state.lastname = last;
    }
  }),
  views: state => ({
    fullname: () => {
      console.log('fullname');
      return `${state.firstname} ${state.lastname}`;
    }
  }),
});

const instance = Person({ firstname: 'Tom' });
instance.setLastName('Clancy');
@ForsakenHarmony
Copy link
Owner

hey, I'm doing alright

I'll look into it, that's definitely too many updates
Sadly can't really do magic and check which props are used in the view

@gabemeola
Copy link

Would it be possible to pass state to each view, then diff and memoize?

@ForsakenHarmony
Copy link
Owner

ForsakenHarmony commented Feb 2, 2018

Was thinking about only computing on access & change

passing the state to each view would change the api

also memoizing with deep state is kinda hard

@ForsakenHarmony
Copy link
Owner

I've released 0.2.1 which should only update the views twice in your example

@tinchoz49
Copy link
Author

it works! :) thanks @ForsakenHarmony! closing the issue

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

3 participants