-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
substate: Add package #28116
substate: Add package #28116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 from me! This fork was merged and released as part of @wp-g2/*
v0.0.133
https://github.com/ItsJonQ/g2/releases/tag/v0.0.133
The zustand
port was something @saramarcondes and I worked on together (mostly Sara. I just flailed around with TypeScript, haha)
Question is, should this just go into compose? Or can it stay as it's own package?
That's a good question. I'm unsure. I'm okay with either, whichever one folks feel more comfortable.
Size Change: 0 B Total Size: 1.3 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ii believe the idea was to add this to the components package temporarily before refactoring to @wordpress/data wasn't it?
(Context for others folks: There was a lot of discussion in the original PR) @youknowriad Hmm! I'm unsure. Taking a step back, we used zustand for because...
These features aren't unique to The discussions swayed towards using something I'm 100% open to refactoring towards a better solution + cleaner convention, but this will take time.
This package is used across all of the other packages in the If it were moved into
Forgive my ignorance! I'm not sure how this relates to ☝️ That's my understanding of things. Hopefully this context helps on your side. I'd love to know your thoughts on the best approaches for organization and dependencies to integrate the |
Hey thanks for the details @ItsJonQ I think the place where we are misaligned is the "packages". To put it clearly, I don't think we should be creating packages for G2 in Gutenberg, at least not in the beginning, I think we should just add everything in folders in the "components" package. The reasons are simple:
Now for @wordpress/data and zustand, I was basing that on @jsnajdr's analyze on the previous PR. I'm fine keeping zustand if it doesn't duplicate what we already have but it's not what the comment suggested. And I believe we can start with it in "components" even if it's a duplication until we have more time to refactor or do something else. |
I was envisioning usage of import { createStore, useSelect, useDispatch } from "@wordpress/data";
const counterStore = createStore({
reducer: (counter = 0, action) => action.type === "inc" ? counter + 1 : counter,
selectors: { get: (state) => state },
actions: { increment: () => ({ type: "inc" }) },
});
function Counter() {
const counter = useSelect((select) => select(counterStore).get());
const { increment } = useDispatch(counterStore);
return <button onClick={increment}>{counter}++</button>;
}
ReactDOM.render(<Counter />, document.getElementById("root")); Here we create a data store that's not registered in any global registry, doesn't have any unique string name, and doesn't need a global provider component. It's completely local to the module. We are not there yet, and won't be there tomorrow or next week, but we already took some steps in that direction in the last 2 months. And the fact that the components package has a clear need for this should only accelerate that development. With such a data store, the overlap with For implementing lightweight stores like this, I think that finishing @adamziel's "async actions" proposal from #27276 would be also very useful. It would make writing actions much more powerful, without having to learn the entire DSL of |
@saramarcondes, thank you for all the work put into G2 components so far. I wanted to echo what @youknowriad shared. We discussed it before and I had the feeling that we are focusing on moving the implementation of individual components for now and keep all lower-level packages as external dependencies of
For some time it should be perfectly fine to keep those packages external. We are comfortable doing it for several well-established low-level libraries like Regarding |
Sounds good to me y'all. I'll close this PR and open a PR for FWIW, @ItsJonQ and I have been working on removing zustand as a dependency. It seems to be possible using a combination of |
Yes, we need to move UI components first. Let’s postpone work on dependant G2 packages until we have several UI components integrated into |
Description
This package is a fork of zustand plus one
useSubState
hook that is a dependency of G2.Question is, should this just go into
compose
? Or can it stay as it's own package?How has this been tested?
Unused so build just has to pass.
Types of changes
New feature/package
Checklist: