Skip to content

[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

Closed
wants to merge 2 commits into from

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Aug 7, 2018

Based on #13293

An updated implementation of simple-cache-provider (final name TBD) using Context.set and Context.read.

  • Subscription support. load accepts a promise or an observable as its return value.
  • Default to LRU cache that is shared across resources
  • Use calculateChangedBits API to optimize updates

Things I'll probably defer until later since they aren't required for an MVP:

  • Support overriding global cache implementation
  • Support overriding cache implementation for a subtree

@pull-bot
Copy link

pull-bot commented Aug 7, 2018

React: size: 🔺+7.3%, gzip: 🔺+5.9%

ReactDOM: size: 🔺+1.1%, gzip: 🔺+0.9%

Details of bundled changes.

Comparing: b3a4cfe...dcec61d

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react.development.js +3.1% +3.1% 59.73 KB 61.57 KB 16.73 KB 17.25 KB UMD_DEV
react.production.min.js 🔺+7.3% 🔺+5.9% 6.98 KB 7.49 KB 2.95 KB 3.12 KB UMD_PROD
react.development.js +3.4% +3.4% 53.92 KB 55.76 KB 14.9 KB 15.4 KB NODE_DEV
react.production.min.js 🔺+8.5% 🔺+6.9% 5.98 KB 6.49 KB 2.56 KB 2.74 KB NODE_PROD
React-dev.js +3.6% +3.5% 51.54 KB 53.38 KB 14.24 KB 14.73 KB FB_WWW_DEV
React-prod.js 🔺+10.1% 🔺+8.3% 13.92 KB 15.33 KB 3.88 KB 4.21 KB FB_WWW_PROD

react-dom

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-dom.development.js +0.5% +0.4% 649.95 KB 653.46 KB 152.57 KB 153.24 KB UMD_DEV
react-dom.production.min.js 🔺+1.1% 🔺+0.9% 95.64 KB 96.66 KB 31.28 KB 31.56 KB UMD_PROD
react-dom.development.js +0.5% +0.4% 646.09 KB 649.59 KB 151.45 KB 152.13 KB NODE_DEV
react-dom.production.min.js 🔺+1.1% 🔺+1.1% 95.64 KB 96.67 KB 31 KB 31.34 KB NODE_PROD
ReactDOM-dev.js +0.5% +0.5% 653.38 KB 656.95 KB 149.63 KB 150.41 KB FB_WWW_DEV
ReactDOM-prod.js 🔺+0.7% 🔺+1.2% 288.67 KB 290.61 KB 53.45 KB 54.08 KB FB_WWW_PROD
react-dom.profiling.min.js +1.1% +1.0% 96.83 KB 97.86 KB 31.4 KB 31.72 KB NODE_PROFILING
ReactDOM-profiling.js +0.7% +1.2% 291.01 KB 292.96 KB 54.05 KB 54.69 KB FB_WWW_PROFILING

react-art

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-art.development.js +0.8% +0.7% 441.73 KB 445.24 KB 99.9 KB 100.56 KB UMD_DEV
react-art.production.min.js 🔺+1.2% 🔺+1.2% 86.62 KB 87.69 KB 26.71 KB 27.02 KB UMD_PROD
react-art.development.js +0.9% +0.8% 374.27 KB 377.78 KB 82.81 KB 83.49 KB NODE_DEV
react-art.production.min.js 🔺+2.1% 🔺+1.9% 51.6 KB 52.68 KB 16.09 KB 16.39 KB NODE_PROD
ReactART-dev.js +1.0% +1.0% 364.66 KB 368.23 KB 77.61 KB 78.39 KB FB_WWW_DEV
ReactART-prod.js 🔺+1.4% 🔺+2.1% 159.84 KB 162.07 KB 27.09 KB 27.66 KB FB_WWW_PROD

react-test-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-test-renderer.development.js +0.9% +0.8% 370.81 KB 374.32 KB 81.3 KB 81.96 KB UMD_DEV
react-test-renderer.production.min.js 🔺+2.1% 🔺+2.2% 50.73 KB 51.8 KB 15.5 KB 15.84 KB UMD_PROD
react-test-renderer.development.js +1.0% +0.8% 366.94 KB 370.45 KB 80.34 KB 81 KB NODE_DEV
react-test-renderer.production.min.js 🔺+2.1% 🔺+2.3% 50.44 KB 51.52 KB 15.34 KB 15.69 KB NODE_PROD
ReactTestRenderer-dev.js +1.0% +1.0% 371.96 KB 375.53 KB 79.16 KB 79.95 KB FB_WWW_DEV

react-reconciler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react-reconciler.development.js +1.0% +1.0% 355.29 KB 358.79 KB 76.87 KB 77.61 KB NODE_DEV
react-reconciler.production.min.js 🔺+2.2% 🔺+2.3% 49.09 KB 50.18 KB 14.72 KB 15.06 KB NODE_PROD
react-reconciler-persistent.development.js +1.0% +0.9% 353.85 KB 357.36 KB 76.29 KB 77.01 KB NODE_DEV
react-reconciler-persistent.production.min.js 🔺+2.2% 🔺+2.3% 49.1 KB 50.19 KB 14.72 KB 15.07 KB NODE_PROD

simple-cache-provider

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
simple-cache-provider.development.js +336.0% +249.5% 8.88 KB 38.72 KB 2.86 KB 9.99 KB NODE_DEV
simple-cache-provider.production.min.js 🔺+410.8% 🔺+299.6% 1.63 KB 8.32 KB 827 B 3.23 KB NODE_PROD
SimpleCacheProvider-dev.js +380.4% +302.0% 7.92 KB 38.03 KB 2.4 KB 9.63 KB FB_WWW_DEV
SimpleCacheProvider-prod.js 🔺+536.5% 🔺+365.8% 3.65 KB 23.21 KB 1.11 KB 5.17 KB FB_WWW_PROD

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-dev.js +0.7% +0.7% 485.92 KB 489.49 KB 107.49 KB 108.29 KB RN_FB_DEV
ReactNativeRenderer-prod.js 🔺+1.1% 🔺+1.6% 214.61 KB 217.02 KB 37.41 KB 38.01 KB RN_FB_PROD
ReactNativeRenderer-dev.js +0.7% +0.7% 485.65 KB 489.22 KB 107.43 KB 108.22 KB RN_OSS_DEV
ReactNativeRenderer-prod.js 🔺+1.2% 🔺+1.7% 204.42 KB 206.83 KB 35.76 KB 36.35 KB RN_OSS_PROD
ReactFabric-dev.js +0.7% +0.7% 476.09 KB 479.66 KB 105.06 KB 105.83 KB RN_FB_DEV
ReactFabric-prod.js 🔺+1.2% 🔺+1.8% 196.83 KB 199.25 KB 34.27 KB 34.88 KB RN_FB_PROD
ReactFabric-dev.js +0.7% +0.7% 476.13 KB 479.7 KB 105.08 KB 105.85 KB RN_OSS_DEV
ReactFabric-prod.js 🔺+1.2% 🔺+1.8% 196.87 KB 199.28 KB 34.29 KB 34.9 KB RN_OSS_PROD
ReactNativeRenderer-profiling.js +1.2% +1.6% 207.85 KB 210.25 KB 36.42 KB 37.01 KB RN_OSS_PROFILING
ReactFabric-profiling.js +1.2% +1.8% 199.83 KB 202.25 KB 34.85 KB 35.46 KB RN_OSS_PROFILING
ReactNativeRenderer-profiling.js +1.1% +1.6% 218.02 KB 220.42 KB 38.07 KB 38.67 KB RN_FB_PROFILING
ReactFabric-profiling.js +1.2% +1.8% 199.79 KB 202.21 KB 34.83 KB 35.44 KB RN_FB_PROFILING

Generated by 🚫 dangerJS

@acdlite acdlite force-pushed the rewrite-scp branch 2 times, most recently from f78b42f to 4fb31b3 Compare August 8, 2018 03:14
@plievone
Copy link
Contributor

plievone commented Aug 13, 2018

This is very exciting work, enabling streaming stores with automatic dependency tracking, granular fetching, still keeping the UI consistent at all times, etc. Sorry if this is too early to ask, but I'm interested in how many contexts will be feasible with this design, are a few hundred contexts/stores ok performance-wise or too much for this design? I'm withdrawing my question, please merge some version. :)

@acdlite acdlite force-pushed the rewrite-scp branch 5 times, most recently from f943fd1 to 3d52ab7 Compare August 17, 2018 01:30
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.
@subtleGradient
Copy link
Contributor

At the risk of adding completely worthless off topic comments...

it's awesome that this PR is #13337 .
So eleeet!

@plievone
Copy link
Contributor

plievone commented Oct 1, 2018

@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.

@jaredpalmer
Copy link
Contributor

Just summarizing these API changes for reference as I wasn't able to find them documented elsewhere:

  • createResource now has support for subscriptions. You return an object with a subscribe key which sets up the subscription, and it can return another fn for teardown.
  • Breaking Change: Resource.read() no longer needs cache instance as first argument, just the input.

New Resource API

  • Resource.preload(input): Preload a value in the cache
  • Resource.read(input): Read a value from the cache

New Cache API

  • useCache(): Get cache via context (calls Context.unstable_read())
  • cache.set(resource, input, value): Set a value in the cache
  • cache.purge(): Wipe the LRU cache
  • cache.invalidate(resource, input): Invalidate an item in the cache, given a resource and input (will run the hash fn, if it was provided to resource)
  • setGlobalCacheLimit(limit): Set global LRU cache size

@acdlite
Copy link
Collaborator Author

acdlite commented Oct 1, 2018

@plievone This branch is the closest to the final version

@jaredpalmer
Copy link
Contributor

@acdlite Why not add Resource.invalidate(input) and Resource.set(input, value) to the Resource API? For basic CRUD stuff, that would avoid having to use the cache directly. It might make the mental model a bit easier too...one less thing to pass around in your application.

function Thing(props) {
 const data = ThingResource.read(props);
 function update() {
   PatchUpdateThing().then(() => {
     // Trigger a reload
     ThingResource.invalidate(props)
   })
 }
 return <Something data={data} refresh={update} />
}

@acdlite
Copy link
Collaborator Author

acdlite commented Oct 1, 2018

@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 connect() instead of reading from the store directly.)

@plievone
Copy link
Contributor

plievone commented Oct 2, 2018

@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.

yarn test passes except one related to react-profiler tracing:

Profiler › interaction tracing › suspense › traces both the temporary placeholder and the finished render for an interaction
Invariant Violation: Context.unstable_read(): Context can only be read while React is rendering, e.g. inside the render method or getDerivedStateFromProps.

yarn lint passes except one error related to flow typings (CacheImplementation is defined but never used)

yarn flow has errors that have been there I guess.

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.

@jaredpalmer
Copy link
Contributor

In my React Conf talk, would it be better to show off code from this PR or what's currently in master?

@acdlite
Copy link
Collaborator Author

acdlite commented Oct 4, 2018

@jaredpalmer Uh, you and I need to chat about your React Conf talk at some point so we can coordinate :D

@jaredpalmer
Copy link
Contributor

Will DM.

mehiel added a commit to mehiel/cra-suspense-starter that referenced this pull request Oct 12, 2018
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
mehiel added a commit to mehiel/cra-suspense-starter that referenced this pull request Oct 12, 2018
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
mehiel added a commit to mehiel/cra-suspense-starter that referenced this pull request Oct 12, 2018
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
mehiel added a commit to mehiel/cra-suspense-starter that referenced this pull request Oct 12, 2018
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
@plievone
Copy link
Contributor

plievone commented Nov 1, 2018

@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?

@zaguiini
Copy link

What's the status of this PR?

@necolas necolas added the React Core Team Opened by a member of the React Core Team label Jan 8, 2020
@acdlite
Copy link
Collaborator Author

acdlite commented Jan 23, 2020

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.

@acdlite acdlite closed this Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants