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

reworked types, methods now on useStore #44

Merged
merged 4 commits into from
Aug 5, 2022

Conversation

funwithtriangles
Copy link
Collaborator

@funwithtriangles funwithtriangles commented Aug 3, 2022

So I was reworking the types (#42) which lead me to some other changes (e.g. having undo, redo, etc, live as static methods (e.g. useStore.zundo.undo())

Not sure if this is the direction you want to go in (or if it clashes with all the stuff you've done with v2) but thought I'd put this up here as a draft.

Currently all stories are broken except for "other middlewares", where you can see the new API

@funwithtriangles funwithtriangles marked this pull request as draft August 3, 2022 15:12
@charkour charkour self-assigned this Aug 3, 2022
@charkour
Copy link
Owner

charkour commented Aug 3, 2022

Sweet, thanks for this!

In v2, I went a similar route like this: useStore.temporal.undo(). Maybe temporal is the wrong word. What do you think?

My other concern is that this PR introduces breaking API changes, so I could release this as a beta npm package in the interim before v2 is released rather than release v1.7.0. What do you think?

@funwithtriangles
Copy link
Collaborator Author

funwithtriangles commented Aug 3, 2022

I think temporal is okay, but there is a risk of clashing with other libraries because it's isn't as specific as zundo.

Totally makes sense to release as a beta version! Let's make sure we're matching on API at least, so a choice between temporal, zundo or something else. Is it worth me fixing up the stories, or will these be completely overhauled anyway?

@charkour
Copy link
Owner

charkour commented Aug 3, 2022

Okay, the only other potential clash would be multiple middleware using the same temporal name to extend the store, right? Maybe we could add a user option to allow them to call it whatever they want.

I prefer temporal because I thought that was the best word to encapsulate the state tracking idea, both undo and redo. Perhaps there is a better word.

No need to update the stories. They will have a major overhaul. The progress of v2 can be found on this branch. I've opted to use TDD instead of storybook to drive the development. Would you want to help cross off some features? I can provide a better description of each bullet point. Right now they just make sense in my head.

@charkour charkour added the enhancement New feature or request label Aug 3, 2022
@funwithtriangles
Copy link
Collaborator Author

The only other word I can think of is history, which is a bit more typical when you're thinking of undo/redo. If it's possible to have an option to change the word in case of clashes, I guess that's the best approach (although I fear typing it might be quite complicated 😅).

I can take a look at devtools support for v2 (e.g. named actions)

@funwithtriangles
Copy link
Collaborator Author

I've opted to use TDD instead of storybook to drive the development.

This makes sense but it could be quite nice to have one story/runnable page to test things like Redux devtools. Otherwise it's a bit of a pain to link the library locally to another local repo.

@charkour
Copy link
Owner

charkour commented Aug 4, 2022

I like the word history, but feel that it implies only the past and not the future. I like temporal because it is a word that mean related to time.

Thanks for being willing to look at dev tools!

This makes sense but it could be quite nice to have one story/runnable page to test things like Redux devtools. Otherwise it's a bit of a pain to link the library locally to another local repo.

I don't have storybook set up, but I do have a way to testing the library visually. I'm re-structuring the repo in v2, but haven't added storybook back yet.

Basic ways to test:

  • pnpm i at root
  • cd packages/zundo and pnpm build
  • cd ../../apps/web and pnpm dev

The apps/web next.js project is similar to storybook where you can visually test it. In the future, the web next.js project will become the examples page, but right now I'm using it for visual testing.

The branch should be in a working state. I'm working on the history depth limit and am fixing a couple bugs regarding when the store should clear. So sorry the development environment isn't in a good spot yet, so I completely understand if you want to wait. My goal is to have the repo cleaned up by the end of the weekend.

@charkour
Copy link
Owner

charkour commented Aug 4, 2022

With this current PR #44, are you wanting me to release a beta for you to use before v2 is done? Just let me know. Thanks!

@funwithtriangles
Copy link
Collaborator Author

Thanks for all this info! Very useful. Currently I am just linking to that branch as a package and so don't really need this as a release (or even this draft PR). I will keep this PR open just in case that fails for some reason. I will close it when all is confirmed and working well.

@charkour
Copy link
Owner

charkour commented Aug 5, 2022

Sounds good. Thanks for the updates and the help.

@funwithtriangles
Copy link
Collaborator Author

So actually it might be useful for us if you could publish this PR as a beta. Unfortunately our CI is failing when trying to pull the package from my Github branch, no idea why. It should be known that this branch is definitely a bit of a mess though!

@funwithtriangles funwithtriangles marked this pull request as ready for review August 5, 2022 13:12
@charkour
Copy link
Owner

charkour commented Aug 5, 2022

Sounds good. Thanks for all the work, @funwithtriangles. I'll give it a review after my work day. Thanks for the patience!

Copy link
Owner

@charkour charkour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great. And I'll release a temporary version for you.

I'll wait to merge this in for now.

@charkour charkour merged commit 272186d into charkour:main Aug 5, 2022
@charkour
Copy link
Owner

charkour commented Aug 5, 2022

This has been released as v2.0.0-experimental.0. Thank you for the help!

You can install with npm i zundo@experimental. I'll continue to work on zundo v2 this weekend.

@funwithtriangles
Copy link
Collaborator Author

Amazing, thanks!

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

Successfully merging this pull request may close these issues.

2 participants