Skip to content

fix(solid-table): converge on use for hooks #5984

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.

Sign up for GitHub

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

Jump to bottom
Closed
birkskyum wants to merge 2 commits into from
Closed

fix(solid-table): converge on use for hooks #5984

birkskyum wants to merge 2 commits into from
+39 −30

Conversation

@birkskyum
Copy link
Member

@birkskyum birkskyum commented Apr 8, 2025 •
edited
Loading

Fixes the redirects in the docs when changing between vue/react frameworks to solid, or vice-versa

Adopt useSolidTable syntax for solid, so it's like vue/react/svelte5. This is aligned with how we do for tanstack router (useParams, useSearch, useRouter ..) and query (useQuery, useMutation ..), store (useStore) and the "official" solid router (useNavigate, useMatch ..), so it's nice to have syntax convergence for hooks.

@nx-cloud
Copy link

nx-cloud bot commented Apr 8, 2025 •
edited
Loading

View your CI Pipeline Execution ↗ for commit c0b51ed.

Command Status Duration Result
nx affected --targets=test:format,test:sherif,t... ✅ Succeeded 1m 2s View ↗
nx run-many --targets=build --exclude=examples/** ✅ Succeeded 6s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-08 16:02:49 UTC

@pkg-pr-new
Copy link

pkg-pr-new bot commented Apr 8, 2025

More templates

@tanstack/angular-table

npm i https://pkg.pr.new/@tanstack/angular-table@5984

@tanstack/lit-table

npm i https://pkg.pr.new/@tanstack/lit-table@5984

@tanstack/qwik-table

npm i https://pkg.pr.new/@tanstack/qwik-table@5984

@tanstack/react-table

npm i https://pkg.pr.new/@tanstack/react-table@5984

@tanstack/match-sorter-utils

npm i https://pkg.pr.new/@tanstack/match-sorter-utils@5984

@tanstack/react-table-devtools

npm i https://pkg.pr.new/@tanstack/react-table-devtools@5984

@tanstack/solid-table

npm i https://pkg.pr.new/@tanstack/solid-table@5984

@tanstack/svelte-table

npm i https://pkg.pr.new/@tanstack/svelte-table@5984

@tanstack/table-core

npm i https://pkg.pr.new/@tanstack/table-core@5984

@tanstack/vue-table

npm i https://pkg.pr.new/@tanstack/vue-table@5984

commit: c0b51ed

@riccardoperra
Copy link
Collaborator

riccardoperra commented Apr 8, 2025 •
edited
Loading

Could be ok for @deprecate, but I'm not sure really about this 🤔. Following a comment by Ryan a while ago , create seems more appropriate in this case.

For example, in @tanstack/store, you're creating a new store via new Store, then you make use of it via useStore. I think the same can be compared with the other libraries. Also solid-primitives follows a similiar approach

In table, createSolidTable will construct the table and create a state there, it's not using something defined on the top.

In order to have more consistent name in v9 (alpha branch) it has been renamed to createTable

@KevinVandy
Copy link
Member

KevinVandy commented Apr 8, 2025

I'm in favor of keeping it as create as per Ryan's comments. As for the TanStack docs switching between frameworks causing 404s, I think we can just set up better redirects

@birkskyum
Copy link
Member Author

birkskyum commented Apr 8, 2025 •
edited
Loading

We'll need the redirects for sure - angular e.g. often has an injectX naming pattern. @KevinVandy , my solution was to have the names be the same, but if you're in favor of adding more advanced redirect logic instead, will you be able to look into it?

I understand the sentiment in Ryan's comment, I just don't agree with it justifying an extra migration step / maintenance churn - especially because the exact same logic of "we're not using, we're creating" does apply to vue and svelte too after their adoption of signals, but both still decided to stick to use anyway. Solid is simply too small to go solo on that imo. Many other choices on the solid api have been made with "let's call it the same as react", to make it familiar, given how virtually all solid devs are prev. react devs. Things like "use server"; are clear examples of that.

@ryansolid
Copy link

ryansolid commented Apr 8, 2025

I see where this came from now. It's interesting. We do use use___ specifically for things that use context. More recently I've been debating having the use signify things that have to live inside of the render tree/reactive graph. Ie.. useContext, useEffect, useUniqueId vs createSignal, createStore, which can be used outside... However the feedback was that was too subtle. That being said it would be very odd at this point I think to be like useSignal.

"use server" is a bit different. It was less the phrasing and the use of those sort of statements we were following suit on.. There is a "use strict" after all.

I guess the question is whether to just say all custom primitives use use. Svelte and Vue don't use use internally to my knowledge so this convention is something coming from third parties. Solid Primitives followed core conventions whereas the ecosystem lead Svelte and Vue who switched to these sort of primitives later on after being established. I don't know this seems like a bigger question than can be answered here.

Of course this is a 3rd party library so you can do whatever you want.

@birkskyum birkskyum closed this Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@birkskyum @riccardoperra @KevinVandy @ryansolid