Skip to content

Commit

Permalink
cache proxy state
Browse files Browse the repository at this point in the history
  • Loading branch information
dai-shi committed Feb 25, 2019
1 parent 563720a commit 23e2c5a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Change Log

## [Unreleased]
### Changed
- Cache proxy state for more performance

## [1.1.0] - 2019-02-17
### Changed
Expand Down
14 changes: 12 additions & 2 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,21 @@ var useReduxState = function useReduxState() {
lastState.current = state;
}); // trapped

var proxyMap = (0, _react.useRef)(new WeakMap());
var trappedMap = (0, _react.useRef)(new WeakMap());
var lastTrapped = (0, _react.useRef)(null);
var trapped = (0, _proxyequal.proxyState)(state);
var trapped;
(0, _react.useEffect)(function () {
lastTrapped.current = trapped;
}); // subscription
});

if (trappedMap.current.has(state)) {
trapped = trappedMap.current.get(state);
} else {
trapped = (0, _proxyequal.proxyState)(state, null, proxyMap.current);
trappedMap.current.set(state, trapped);
} // subscription


(0, _react.useEffect)(function () {
var callback = function callback() {
Expand Down
10 changes: 9 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,19 @@ export const useReduxState = () => {
lastState.current = state;
});
// trapped
const proxyMap = useRef(new WeakMap());

This comment has been minimized.

Copy link
@theKashey

theKashey Feb 25, 2019

That would be cool to have proxy mapping as a top-level variable, so different useReducers could benefit from it.
(requires guaranteed synchronous code to remove any races, hooks, probably, guarantee it.

This comment has been minimized.

Copy link
@dai-shi

dai-shi Feb 25, 2019

Author Owner

Yeah, I thought about that too. I wasn't very confident. Will try.

This comment has been minimized.

Copy link
@theKashey

theKashey Feb 25, 2019

I am also not quite confident. It's easy to break something, not fix.

This comment has been minimized.

Copy link
@dai-shi

dai-shi Feb 27, 2019

Author Owner

I didn't try coding, but just thoughts. I don't think the global proxyMap would work.
That's my original question/example in theKashey/proxyequal#12
The affected is in the closure (?) of a cached proxy and you can't create a new trapped.

This comment has been minimized.

Copy link
@theKashey

theKashey Feb 28, 2019

It could be "fixed", but might make "normal" cases less performant.
There a way to mitigate it, more about calling .reset on proxy and reading .affected from the same variables

This comment has been minimized.

Copy link
@dai-shi

dai-shi Feb 28, 2019

Author Owner

The purpose was to share proxy objects for different useReducers, so calling .reset() may not work, if I understand correctly.

This comment has been minimized.

Copy link
@theKashey

theKashey Feb 28, 2019

  • get update from a store
  • call reset
  • read all you need
  • consume .affected

Would work for synchronous code... without very closures, which may be called out of the main cycle. But you are not handling this case today.

This comment has been minimized.

Copy link
@dai-shi

dai-shi Feb 28, 2019

Author Owner

Oh, I see. Yeah, it seems to work for synchronous code. Not for this project, because we can't tell when a render function is called. (I don't think it would work even if we useLayoutEffect.)

Well, I'll look for somewhere else for improvements...

const trappedMap = useRef(new WeakMap());
const lastTrapped = useRef(null);
const trapped = proxyState(state);
let trapped;
useEffect(() => {
lastTrapped.current = trapped;
});
if (trappedMap.current.has(state)) {
trapped = trappedMap.current.get(state);

This comment has been minimized.

Copy link
@theKashey

theKashey Feb 25, 2019

Yep, that would solve the problem!

} else {
trapped = proxyState(state, null, proxyMap.current);
trappedMap.current.set(state, trapped);
}
// subscription
useEffect(() => {
const callback = () => {
Expand Down

0 comments on commit 23e2c5a

Please sign in to comment.