feat: createDataStore() hook#217
Conversation
|
Thanks for the PR! How about we keep |
|
Done and updated the title: now it's a feature and not a breaking change (I think that's the intention) |
|
Since it basically just wraps Solid's But for React it would clearly be called Hm. |
c12a349 to
ce96c27
Compare
ce96c27 to
1375088
Compare
You're right.
Because
Other alternatives are to use IMO I will remain with @magne4000 @phonzammi FYI and if you want to help/recommend/contribute |
|
IMO I still think In the meantime, we can go ahead an create a vike-react and vike-vue PR for creating |
Or
It's referred only to the renaming? If yes, I will wait a decision (maybe after other opinions) first at Solid side (this PR). |
|
👍 Let's wait a bit and then proceed. |
|
Actually, thanks to @phonzammi's work on DocPress (most notably brillout/docpress#73 and brillout/docpress#91) naming it differently for React/Vue/Solid won't be that much of a problem for Vike's docs. So I'd stay:
|
|
Claude suggests naming it So:
Any objections? |
|
Actually... I think we can go with:
Because with TypeScript it becomes @rtritto I definitely agree with you that the more succinct the name of commonly used APIs the better (a lot easier to remember). Thank you for pushing on this! WDYT? Let's go with this? |
This naming conflicts with what exists in the framework, eg: import { createStore } from 'solid-js/store'
import { createStore } from 'vike-solid/useData'
// Same namingThis is bad (it's not nice) because the user is forced to use an alias for 1 of the 2 and a search can find both. IMO, I would use:
Data refers that the value arrives from So DataType is shared with // /pages/+data.ts
export const data = () => {
...
return {
...
} satisfies DataType
} |
That's a good point... you're right.
I think, often (most of the time?), the data is static after it's fetched (no client state) — there isn't any need to create a state. For this simple use case, the current That's why I'm inclined to define two different hooks:
One could argue that Basically, we have to choose between two different paper cuts. WDYT? |
This is a design choice; the concept of multiple hooks is similar to
There's a discussion for Jotai v3 and React v19, maybe can help to choose. From my point of view, instead of using The result will be:
In the end the design choice is between:
Both has pros and cons. |
Makes sense!
Let me mull over it a bit 👀 |
|
How about |
|
Yes, it's better (compact and readable), LGTM |
|
💯I’m glad we found a sustainable name. Great constructive discussion. |
ed26d30 to
6358ee2
Compare
6358ee2 to
46b892f
Compare
|
I wonder whether we can dedupe some code between |
It isn't much code, so not sure whether that's worth it. |
|
Do you mean in the examples? - import { createDataStore } from "vike-solid/createDataStore";
+ import { useData } from "vike-solid/useData";
- const [movie] = createDataStore<Data>();
+ const movie = useData<Data>(); |
|
I meant that the |
// useData.tsx
export { useData };
import { createDataStore } from "./createDataStore.jsx";
/** Access `pageContext.data` from any SolidJS component
*
* See
* - https://vike.dev/data
* - https://vike.dev/pageContext-anywhere
*/
function useData<Data>(): Data {
const [data] = createDataStore<Data>();
return data;
}WDYT? |
|
LGTM. Also I'd say let's move the comment that refers #114 to |
The implementation is inside |
|
0163ba5 — let me know if you disagree. Let's create a PR for |
I would only change like this: - // We use a store to sync the store when the navigation (and thus `pageContext.data`) changes.
- // https://github.com/vikejs/vike-solid/issues/114
+ // https://github.com/vikejs/vike-solid/issues/114
+ // We use `createEffect` to sync the store when the navigation (and thus `pageContext.data`) changes. |
|
👍 Feel free to push |
|
In Vue you should be able to set data directly by reference: const data = useData()
// Update/set data
data.value.title = "New Title"
{{ data.title }}So the In the end, we have:
Note: IMO, in Solid, the @brillout WDYT? |
Good point and I think you're right. (Although I also ain't that familiar with Vue. We should test this.) Note that But we do plan to use So I guess merging that PR will do the trick for Vue?
See https://chatgpt.com/share/69934df1-d3c0-800d-8c83-0d383b2489bc and its response to my question Concretely: isn't Wouldn't the user think "such a complex API for such a simple need: I just want to access data, why do I need to create a store just to access my data"?
I don't see why |
#215 will change behaviour
The main concept is provided by these points:
As result (all combinations) we have:
That's why the get of Solid should be
I agree. I was referring to the subtlety of comparing React hooks (
On Solid side, just create the minimum required hook with
The Store is needed due to Solid's static behavior that's can be changed with If we want to omit the implementation, we could choose (like Jotai's naming):
It's referred to dedupe: reuse the code (better maintainability) vs rewrite the code (raw code for performance). The performance is negligible, there's no need to pay attention to it. |
VueHow about this?
See also: https://vuejs.org/guide/essentials/reactivity-fundamentals SolidSolid actually has I think I don't think the users needs to know that
I don't understand this sentence. Sum up
I don't see why you think this would be better. I think So, on my side, I'm voting for:
I like it. But I'm open to alternatives. |
What about:
Maybe I'm wrong, WDYT?
Ok, so in Solid we will use
Because it's simple: the |
AFAICT it wouldn't add any added value compared to
I'm inclined to think that:
If 80% (just a guess, I could be wrong) of the use cases are simple (only get), then that's also an argument to go for the the simplest name possible for the most common use case. I think read-only +data (only get) is (a lot?) more frequent than read&write +data (get + set). |
Rename
useDatatouseStoreWithData.Note: The return type of the hook was changed from
Datato[Data, set<Data>].Related
useDataState()hook vike-react#214