-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[WIP] Try/fresh data as data plugin #8754
[WIP] Try/fresh data as data plugin #8754
Conversation
This adds fresh-data functionality to @wordpress/data through a `registerApi` function (in lieu of `registerStore`, `registerActions`, etc.) And using a new `withResources` HOC.
This adds mutations to the `registerApi` options, and also a `mapMutationsToProps` function to the `withResources` HOC.
Use the new plugin mechanism within `@wordpress/data` to try out fresh-data to support advanced data features.
While this seems like a good use-case for a data plugin (and a stress test of its capabilities), could you explain for what reason this should be considered for inclusion in the core module itself, vs. distributed separately? How is this planned to be used in a core development context? How does it relate to similar efforts like As it stands, it seems to be a very isolated approach and it's not clear if / how this would be introduced into core development patterns. If it's not intended for use in core features, this is precisely the sort of thing which could make sense as an externally-referenced plugin (e.g. |
Honestly, other than the benefits of a data system like this, I don't have any direct reasons.
If by "core" you mean WordPress core and Gutenberg, I'm not qualified to answer that as I haven't been part of the core development team for Gutenberg. However, for larger, data-dependent applications like WooCommerce, there is definite desire to use this for the new dash work and beyond.
There is overlap, but
This might be tricky. I've found in order to support a concept of freshness well, the freshness requirements need to be driven by the application code (read: application components). This means more than one component may require a resource, but with different freshness requirements (e.g. A large list may only need them fresh to within 1 hour, while an edit screen may need it much more fresh.) This isn't really possible without mapping resource requirements to components, where they can be reduced to a single set of requirements and be updated every time a component mounts or unmounts. And unfortunately, no component context is passed via In summary: If a demand-based freshness concept is to be introduced in
While I really don't agree with the "isolated approach" comment, I do agree that there is a fundamental difference between this approach and what is currently in I guess the question is: Will Personally, I'm comfortable using this as an external plugin as long as we have the relevant teams' blessings to use this in a project like |
Aside: I see that the comment has since been edited, but I sense that there was some offense taken at the use of the phrase "isolated approach". My intention was to summarize that it should be a requirement of this proposal to consider its interoperability with existing tooling (for which there are overlapping goals) or a proposal for how we'd consider to migrate those toward this alternative solution. A goal of |
Re: the "isolated approach" comment, I think it's pretty easy to take that language as a dismissive judgement. (i.e. not a team player, etc.) but I know you didn't mean it that way. I didn't want to communicate it as such, so I edited my response. "Isolated" is much stronger language than "outside of the
This was the primary intent of this PR, in fact. It's a first step in interoperability between the two data systems.
I figured it might be a bit premature to put a lot of work toward something that may not even be desired. I put forth this PR to gauge interest in including it in I'm happy to work on reconciling these two different approaches if it's something that the maintainers of this project would want. I'm only slightly less happy to keep it as an external plugin, so long as my teammates can use it for their work. 😄 |
This PR is no longer needed with the |
Note: This PR is not yet fully baked and ready to merge. It still needs further discussion.
Description
This adds
fresh-data
functionality to@wordpress/data
through the pluginuse
mechanism to adjust the registry in the following ways:registerStore( reducerKey, options )
to be called withapiSpec
as an option, which corresponds to theapiSpec
object infresh-data
.getApiClient( reducerKey )
function to the registry.This PR also adds a
withResources
HOC which is similar to thewithSelect
HOC. It's different because it needs a mapping to the component which is requiring the resource, and I really want to set the requirements of a component in one single operation, rather than clearing the requirements then re-adding them, which can get messy.The reasoning behind this addition is to support more advanced asynchronous API data needs like for the WooCommerce API and other applications which are data-heavy.
This adds the following functionality that does not presently exist in
@wordpress/data
:includes
param with ids).This mechanism still also supports the same features we know and love with
@wordpress/data
such as:Implementation details:
This implementation lays out a mapping of connected components and their data requirements. It then reduces those requirements into an actionable list of resources to be fetched. The mapping of components is critical here, because it allows the resource requirements list to be updated in accordance to component lifecycle. For instance, when a component mounts, it requires the data it needs as referenced by its props (e.g. post #25). When it unmounts, those requirements are cleared from the system and no further fetching of that data will occur (unless there's another component requiring it too, of course). It's by this mechanism that duplicate fetches are avoided without resorting to debouncing. Only one part of the code ever fetches data, and it does it in accordance with the complete mapping of all data required for that api.
Further notes:
Through this process, I've discovered some places where
fresh-data
can be simplified without reducing functionality. These changes will also provide closer integration points to the@wordpress/data
code. I am working on these changes and intend to update this code to work with them after they are all merged.Integration:
This functionality is necessary for more complex API applications. However, I'm open to discussion about how best to integrate these changes. Let's have a discussion about it.
Also, with the addition of the plugin mechanism, it should be possible for this code to exist entirely outside of the
@wordpress/data
package if so desired. (with one caveat:RegistryConsumer
would need to be exported from@wordpress/data
for the HOC to be possible.)How has this been tested?
I created a PR in
wc-admin
which uses@wordpress/data
as a dependency and implements some WooCommerce API calls: woocommerce/woocommerce-admin#248See the instructions on that PR for testing.
Types of changes
plugins/fresh-data
to@wordpress/data
exports.fresh-data
plugin addsapiSpec
andgetApiClient
to registry.withResources
HOC.No breaking changes.
Checklist: