-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Draft] Data layer overview #37615
base: trunk
Are you sure you want to change the base?
[Draft] Data layer overview #37615
Conversation
A comment I found in my notes:
|
I guess the most important thing for me first, is to figure out where this fits and how it aligns with the existing data related docs. Is this a README of the data module? Is this an "architecture" doc to put under the architecture section? Is it specific to core-data? Should we split it? I want to avoid the effect of just adding docs that duplicate some existing docs, and adds good additions... but at the same time confuses the reader. I guess I'm just saying we should think about organization and consolidate with existing documentation. |
I echo @youknowriad's comment - we should try to find a good spot and think how it fits with the rest. My sense with the way it is now would be the explanations/architecture section. The Gutenberg documentation is structured based on this documentation system with the main sections being:
|
One thing I will add here - I believe this documentation is critical and I applaud all the effort that has clearly gone into this. We get a lot of questions from contributors (even those who are otherwise well versed in the project!) on the data layer. I agree we should decide where this fits. I'm going to give it a read and leave some thoughts. |
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.
I got this far a review. I'll tackle the rest tomorrow or early next week.
docs/getting-started/data.md
Outdated
null | ||
``` | ||
|
||
First, `getTemperatureCelcius` does not refer to the same function as we originally registered with the store (`( state ) => state.temperature`). Instead, the `data` package replaced it with a „resolved” version using the [mapResolvers](https://github.com/WordPress/gutenberg/blob/5dbf7ca8a285f5cab65ebf7ab87dafeb6118b6aa/packages/data/src/redux-store/index.js#L366-L442) utility. The function we’re actually calling is [`selectorResolver`](https://github.com/WordPress/gutenberg/blob/5dbf7ca8a285f5cab65ebf7ab87dafeb6118b6aa/packages/data/src/redux-store/index.js#L388) . It does two things: |
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.
...the same function as we originally registered
Are we referring to the selector or the resolver here?
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.
Thanks for writing this up Adam! I agree that more info on how our data packages work is needed to help guide contributors. I'm also not clear on where this should live, but here are some thoughts:
-
As this stands, it reads to me more like a blog post than a technical doc (especially the sections on entity records). Publishing it as a series of blog posts is definitely a possibility! With the data package at least we're covering similar ground to @nerrad 's series on the subject, but it's great to have multiple takes on the same topic, because different people will find one or the other easier to understand.
-
It would be nice to have some docs covering the architecture of these packages in the packages themselves, for the benefit of potential contributors. Having the docs live close to the code has the advantage that searching for a keyword (e.g. a function name) in the project will bring up the doc, which might be helpful.
-
If we want this to be an architectural doc, it should be as short and to the point as possible, to make it easy to browse through and find the needed info (because we all know devs love to read long docs, right? 😅 ) so we'd need to do some editing work here. In this case we should also build as much as possible on the existing docs, improving them where needed and linking to them instead of duplicating effort.
Hope this helps!
docs/getting-started/data.md
Outdated
The `@@INIT` action is dispatched when the store is instantiated, and so the initial state says `temperature: 0` | ||
|
||
### Simple selectors | ||
The `getTemperatureFarenheit` is a simple selector, it predictably returns `0` once the store was instantiated: |
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.
Not sure I understand this example. The selector structure led me to assume the temperature was stored in Celsius. If the initial temperature is 0, won't that be 32 in Farenheit?
We already have a decent overview of how selectors etc. work here so perhaps we should just reference that example?
docs/getting-started/data.md
Outdated
} | ||
``` | ||
|
||
It is a [thunk](thunks%20link) that populates the state. Note that it does not return anything, nor there are any assumptions on how the data is loaded. The sole goal of this function is to populate the state, and it does so by dispatching the `receiveTemperature` action when the data is ready. |
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.
...nor are there any assumptions...
docs/getting-started/data.md
Outdated
|
||
## Entities | ||
|
||
Entities are like data types. A Post is an entity, so is a Taxonomy and a Widget. We will use the latter as our running example. Default entities are declared in `entities.js`, and a minimal definition looks like this: |
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.
"Entities are like data types" sounds a little vague. How else can we define them? Something like "objects" or "types of content" maybe?
Because each entity corresponds to a REST API endpoint, might it be helpful to add that entities are equivalent to REST API "resources"?
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.
I like the REST metaphor! How about something like below? I used three different examples to have a better chance of shoehorning this on top of some prior knowledge the reader has.
An entity is a basic unit of information in core-data. Entities are conceptually similar to REST API resources, database entries, and class objects. A Post is an entity, so is a Taxonomy and a Widget.
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Thank you so much for your reviews @tellthemachines and @getdave, it's extremely helpful! As I was editing, I noticed I have two competing priorities here:
I am thinking about splitting this into two separate docs:
I'm curious about your thoughts @youknowriad @mkaz @draganescu @getdave @tellthemachines |
This is a first draft of a Gutenberg data layer overview. I hope it will help new contributors get up to speed faster. As a follow-up, I would prepare an actual tutorial on building a small app using the material covered here.
There's still a lot I want to change, but I'm posting it here for early feedback anyway. It's easier to improve now than it will be later.
cc @youknowriad @jsnajdr @noisysocks @talldan @draganescu @ntsekouras @jorgefilipecosta @ellatrix @kevin940726 @tellthemachines @priethor @mtias