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

review undo-peasy? #591

Open
mighdoll opened this issue Oct 30, 2020 · 5 comments
Open

review undo-peasy? #591

mighdoll opened this issue Oct 30, 2020 · 5 comments

Comments

@mighdoll
Copy link
Contributor

@ctrlplusb would you or another committer familiar with easy-peasy take a peek at undo-peasy? I’ve just published a 0.2.2 version with better performance and more comments. When it's good to go, it's ok with me to put a link to undo-peasy in the community section of the easy-peasy docs (as you requested in #533).

I have a few questions:

  • Is there going to be a 4.1 release of easy-peasy? It would make sense to publish and test undo-peasy against that version or a later one. My upstream app failed with easy-peasy 4.0.1, but is working fine with easy-peasy 4.1.0-beta.4.

  • Will undo-peasy obviously break some advanced feature of easy-peasy? e.g. hot-reloading.

  • For the future, what do you think about exposing a way to remove computed fields from state in the easy-peasy api?
    undo-peasy jumps through some hoops to calculate and cache a list of computed properties in a model and then remove computed properties before persisting a state to undo history. But easy-peasy has internal functions that would make those hoop jumps unnecessary.

@redbar0n
Copy link

redbar0n commented Jul 7, 2021

@ctrlplusb @crissdev @christianchown @shoeman22 could either of you take a look?

@jmyrland
Copy link
Collaborator

Hi @mighdoll ! I'm a new collaborator for easy-peasy and I'm currently combing through the list of github-issues.

Is there going to be a 4.1 release of easy-peasy? It would make sense to publish and test undo-peasy against that version or a later one. My upstream app failed with easy-peasy 4.0.1, but is working fine with easy-peasy 4.1.0-beta.4.

Sorry for the late response 😅 Is it still broken for the latest version?

Will undo-peasy obviously break some advanced feature of easy-peasy? e.g. hot-reloading.

The API is considered stable for now, so if it works with the latest version, it should work for the forseable future. (Correct me if I'm wrong @ctrlplusb)

For the future, what do you think about exposing a way to remove computed fields from state in the easy-peasy api?
undo-peasy jumps through some hoops to calculate and cache a list of computed properties in a model and then remove computed properties before persisting a state to undo history. But easy-peasy has internal functions that would make those hoop jumps unnecessary.

What internals would you like to access? @ctrlplusb has to look at this in this case.

When it's good to go, it's ok with me to put a link to undo-peasy in the community section of the easy-peasy docs (as you requested in #533).

That would be nice! Please consider adding a small example as well 👍

@mighdoll
Copy link
Contributor Author

undo-peasy tests pass on the latest version of easy-peasy. I will bump the dependencies and republish undo-peasy.

undo-peasy uses unstable easy-peasy implementation internals to find computed properties. see e.g. https://github.com/mighdoll/undo-peasy/blob/master/src/Utils.ts#L78. Probably easy-peasy should publish an api instead to enable this kind of thing to be done externally more stably.

(And please feel free to take anything useful from undo-peasy if you'd like to build undo/redo within easy-peasy.)

@mighdoll
Copy link
Contributor Author

published undo-peasy 0.6.3 with updated dependencies

@jmyrland
Copy link
Collaborator

undo-peasy tests pass on the latest version of easy-peasy. I will bump the dependencies and republish undo-peasy.

Awesome, thanks for checking this.

undo-peasy uses unstable easy-peasy implementation internals to find computed properties. see e.g. https://github.com/mighdoll/undo-peasy/blob/master/src/Utils.ts#L78. Probably easy-peasy should publish an api instead to enable this kind of thing to be done externally more stably.

I see! I think you got a good solution there. For now, I don't think the demand is high enough for exposing this kind of utlis from the lib itself. What is your thoughts on this @ctrlplusb ?

(And please feel free to take anything useful from undo-peasy if you'd like to build undo/redo within easy-peasy.)

Appreciate it 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants