-
Notifications
You must be signed in to change notification settings - Fork 48.6k
[simple-cache-provider] New implementation #13337
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
Conversation
React: size: 🔺+7.3%, gzip: 🔺+5.9% ReactDOM: size: 🔺+1.1%, gzip: 🔺+0.9% Details of bundled changes.Comparing: b3a4cfe...dcec61d react
react-dom
react-art
react-test-renderer
react-reconciler
simple-cache-provider
react-native-renderer
Generated by 🚫 dangerJS |
f78b42f
to
4fb31b3
Compare
|
f943fd1
to
3d52ab7
Compare
Adds a method to update a context's default (global) value. This schedules an update on all context consumers that are not nested inside a provider, across all roots and renderers. For most use cases, this replaces the need to inject a context provider at the top of each root. (I've added an `unstable_` prefix until it's ready for public release.)
First in a series of commits. createResource's load function accepts either an observable or a promise. If an observable is returned, each new value will trigger a re-render. Updates are implemented using the Context.set API. Reads are guaranteed to be consistent across the entire tree.
At the risk of adding completely worthless off topic comments... it's awesome that this PR is #13337 . |
@acdlite, I would like to start experimenting with simple-cache-provider. Would it make sense for me to rebase this implementation of yours on master or just use the existing version? This one has subscriptions etc. |
Just summarizing these API changes for reference as I wasn't able to find them documented elsewhere:
New Resource API
New Cache API
|
@plievone This branch is the closest to the final version |
@acdlite Why not add function Thing(props) {
const data = ThingResource.read(props);
function update() {
PatchUpdateThing().then(() => {
// Trigger a reload
ThingResource.invalidate(props)
})
}
return <Something data={data} refresh={update} />
} |
@jaredpalmer Short answer: the cache is contextual, not global. Longer answer: in practice the cache you read from context is almost always the global one. But if you start with a global API, then later want to render your component in a contextual setting, now you have to refactor all your components to the contextual form. Requiring the contextual version always adds a little bit of typing overhead but it avoids the refactor hazard. (Similar reasoning to why Redux components use |
@acdlite Here is a rebased version, feel free to pull into your workflow: https://github.com/plievone/react/tree/rewrite-scp-rebase2 I aimed to keep your whitespace, commented code, etc. The two commits are there, as you have different pull requests with review going on for those.
I also updated the suspense fixture, and had to fix one trace call missing performance.now() to test cache purging. But seemed to work now in quick testing. So there's some work to do to fix typings, failing test, addressing comments (renaming things, reimplementing things perhaps), but hopefully this is a quicker starting point now. EDIT: rebased again on master to get scheduler fixes. |
In my React Conf talk, would it be better to show off code from this PR or what's currently in master? |
@jaredpalmer Uh, you and I need to chat about your React Conf talk at some point so we can coordinate :D |
Will DM. |
1. react,react-dom@16.6.0-alpha.8af6728 are now published on npm with Suspense provided by default so there is no need for local versions 2. react-cache is not yet released on npm but since it is still api compatible with simple-cache-provider@0.10.0 we can use it (api will change per facebook/react#13337) 3. as long as we use simple-cache-provideri, react-async-elements@0.4.0 works without adding unnecessary dependencies
1. react,react-dom@16.6.0-alpha.8af6728 are now published on npm with Suspense provided by default so there is no need for local versions 2. react-cache is not yet released on npm but since it is still api compatible with simple-cache-provider@0.10.0 we can use it (api will change per facebook/react#13337) 3. as long as we use simple-cache-provideri, react-async-elements@0.4.0 works without adding unnecessary dependencies
1. react,react-dom@16.6.0-alpha.8af6728 are now published on npm with Suspense provided by default so there is no need for local versions 2. react-cache is not yet released on npm but since it is still api compatible with simple-cache-provider@0.10.0 we can use that (api will change per facebook/react#13337) 3. as long as we use simple-cache-provider, react-async-elements@0.4.0 works without adding unnecessary dependencies
1. react,react-dom@16.6.0-alpha.8af6728 are now published on npm with Suspense provided by default so there is no need for local versions 2. react-cache is not yet released on npm but since it is still api compatible with simple-cache-provider@0.10.0 we can use that (api will change per facebook/react#13337) 3. as long as we use simple-cache-provider, react-async-elements@0.4.0 works without adding unnecessary dependencies
@acdlite I wanted to experiment with hooks too, so here's the latest rebased version of your PR: https://github.com/plievone/react/tree/rewrite-scp-rebase2 I aimed to keep your whitespace, commented code, etc, but changed some lines according to changes in master in the meantime. The two commits are there, as you have different pull requests with review going on for those. yarn test passes. When rebasing, I converted the added react-cache tests to use react-test-renderer to pass, except a couple of tests that I couldn't make pass with react-test-renderer so kept them in a separate file with react-noop-renderer. yarn flow has errors, haven't touched those. I have been enjoying experimenting with this API (especially automatic updates when changing/invalidating cache keys). To perhaps continue from this rebased code (for example the converted tests might help you), would you like me to create pull request somewhere? |
What's the status of this PR? |
Will likely ship some version of this package as part of initial Concurrent Mode release, but not this one in this PR. Closing for now. |
Based on #13293
An updated implementation of simple-cache-provider (final name TBD) using
Context.set
andContext.read
.load
accepts a promise or an observable as its return value.calculateChangedBits
API to optimize updatesThings I'll probably defer until later since they aren't required for an MVP: