Entity records hooks #38135
Entity records hooks #38135
Conversation
adamziel
added
[Package] Core data
|
Size Change: +868 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
|
I have recreated a similar hook in past projects and would love to see a core implementation. I have called mine |
Thanks for speaking out @spencerfinnell! That hook name rings a bell, too – see this related PR: #38134 |
There was a problem hiding this comment.
While we're at it. I think this PR (and #38134) is a good candidate to start using TypeScript?
|
It's a very interesting proposal, @adamziel. Thank you for opening the PR that explains the idea. It would be helpful to update a few existing components to present how this impacts the developer experience. By the way, have you explored how often these selectors are used together in the Gutenberg project or how likely that plugins would need the new API proposed?
I think naming here makes a big difference. When I see |
|
By the way, have you considered using Proxy API to make updates to the record simpler to code? Immer or Valtio would be a good example of using immutability for mutable state 😄 Instead of dispatching const { editedRecord } = useEntityMutation( 'postType', 'page', pageId );
function onEdit() {
editedRecord.title = "New title";
editedRecord.content = "New content.";
}The only concern here would be that it makes batching updates a bit more complex to handle. Anyway, just an idea since we talk about a completely new hook. |
I think
Paired with #27859, #38134 (comment) developers would have a very good starting place to consume custom entities through custom REST API endpoints, which has been a requirement for the majority of things I've worked on within the block editor or block editor-style admin page. |
There was a problem hiding this comment.
I really really like the motivation to simplify working with the data layer. At the moment it requires too much overhead and this is a good move towards making it a lot more accessible.
👏👏👏
My personal opinion is that we should keep things simple for now and just run with creating these new hooks. We could consider making them experimental just in case we want to build space for tweaking the APIs in response to feedback on the Plugin.
| ); | ||
|
|
||
| return { | ||
| ...mutationFunctions, |
There was a problem hiding this comment.
Looking at the resulting API I'm wondering whether it might make for more readable components if we instead expose an object on a mutations property.
That would mean we'd call mutations.edit() rather than just edit().
Just an idea.
There was a problem hiding this comment.
There's much less data coming out of this hook now – do you think that removes the need for the additional nesting level (edit)?
adamziel
force-pushed
the
propose/use-entity-mutation
branch
from
4d68560 to
5948110
Compare
|
I tweaked this PR and ended up with the following read hooks:
...and useQuerySelect from #38134 #38289 showcases how they make the navigation block ~200 lines leaner. There are also two write hooks:
I didn't find a good place to showcase them, though. We only save things in a handful of places, and most of the time it's in a redux action and not inside of a React component. I still think these two make valuable additions – perhaps I'll write a made-up example to demonstrate the difference. |
There was a problem hiding this comment.
#38289 showcases how they make the navigation block ~200 lines leaner.
This is super helpful. I appreciate the efforts to put this PR together 🙇🏻
useEntityRecord is used only once, but useEntityRecords 7 times.
It raised the question of whether the existing useEntityProp got popular because it's so convenient or because using getEntityRecord through the data store was hard to use because it's so low-level. Anyway, it's a clear sign that we need those APIs.
I didn't find a good place to showcase them, though. We only save things in a handful of places, and most of the time it's in a redux action and not inside of a React component. I still think these two make valuable additions – perhaps I'll write a made-up example to demonstrate the difference.
Interesting, does it mean we don't need those two hooks at the moment for usage inside Gutenberg? It's a signal that maybe we don't need to rush with that part.
Have you considered whether useEntityRecord could be combined with useEntityRecordCreate and useEntityRecordUpdate? useEntityRecordUpdate handles edit and delete, so there could be also the question whether we need useEntityRecordDelete for consistency. I'm not sure yet what would be better from the user's perspective. One thing is clear if a single component displays data from the record, and you can change that data with a callback, then you would prefer to use now useEntityRecordUpdate which sort of makes useEntityRecord less useful.
| import { IDLE, SUCCESS, RESOLVING } from './constants'; | ||
| import { store as coreStore } from '../'; | ||
|
|
||
| export default function usePermissions( resource, id ) { |
There was a problem hiding this comment.
Nit: for consistency, it might use a more explicit name like useEntityRecordPermissions since it works only for a single record.
What's the story for multiple records?
| canCreate: create.data, | ||
| canUpdate: update?.data, | ||
| canDelete: _delete?.data, |
There was a problem hiding this comment.
Based on the usage in #38289, it might be convenient to assume that permissions return value value only when resolved:
| canCreate: create.data, | |
| canUpdate: update?.data, | |
| canDelete: _delete?.data, | |
| canCreate: hasResolved && create.data, | |
| canUpdate: hasResolved && update?.data, | |
| canDelete: hasResolved && _delete?.data, |
This way you still have granular control over the status but you don't need to guard values.
There was a problem hiding this comment.
I think this makes sense. We should assume that users don't have permission until a query is resolved.
| const count = useSelect( ( select ) => { | ||
| return select( 'core/block-editor' ).getBlockCount(); | ||
| }, [] ); | ||
| const count = useSelect( ( select ) => { |
There was a problem hiding this comment.
Nit: it looks like you formatted all markdown files using a different tool or Prettier config.
| kind, | ||
| type, | ||
| id, | ||
| options = { runIf: true } |
There was a problem hiding this comment.
It looks like a special case that could be handled outside the hook. Can you explain the rationale behind runIf and how do you envision its application?
Just taking a look at this for the first time. My initial instinct, given the names of these hooks are styled in a CRUD fashion, is that there would also be a I'm wondering if there are a lot of use cases where the code for deleting something will live alongside the code for editing and saving the edited value. The other bit of feedback is that it'd be great if these hooks could work the same way as gutenberg/packages/core-data/src/entity-provider.js Lines 96 to 98 in 8fcd7f2 It's a nice little convenience that helps avoid passing ids or slugs around. |
|
Thank you for all the reviews! I propose we move forward in a series of atomic PRs focused on one feature at a time. It's been great to explore and diverge in a single place, and now smaller PRs should make it easier to converge on a solution. The first smaller PR is for the |
gziolo
added
[Type] Technical Prototype
|
|
|
I also created a separate PR for |
|
Thank you for the update @adamziel. Let's make it clear that all those APIs start as experimental so it's |
|
I see that @adamziel opened #39595 that could use broader feedback:
The idea would be to add more functionality to |
|
Any work left on this PR? We’ve shipped a few proposals already. |
|
@gziolo There are two more PRs in motion:
I just refreshed the first one, and the second one still requires a rebase. |
|
This PR has been fully merged through a series of smaller PRs as explained in the description. Let's close it. Thank you for all the feedback and support! |
Description
This PR proposes a new hook called
useEntityMutationinspired by react-query and @talldan and @kevin940726 feedback. It would used like this:I'd like to make core-data more approachable. Building a simple form currently requires a snippet like this:
It is verbose and confusing for new developers. Even knowing about all these functions requires some prior expertise. Having a single entry point to working with entities makes core-data more approachable and turns the challenge from how do I even start? to how to use the available parts?
Together with the upcoming data layer documentation, it could lower the barrier of entry for new contributors, and make the life of existing contributors easier.
This large PR was split into smaller easily mergable PRs:
cc @kevin940726 @talldan @gziolo @draganescu @ellatrix @noisysocks @jsnajdr @getdave