Discussions about rest operator
dai-shi opened this issue ยท 32 comments
@theKashey said:
With this approach, you are just "reading" from state inside component, as you wanted to do, and no need to rest.
We still want to support the rest operator if possible. Or at least provide a workaround.
I haven't looked into proxyequal's spread guards. Is it related to the rest operator?
So far, what I could think of is the following.
const used = { current: {} };
const PROXY_REST = Symbol('PROXY_REST');
const handler = {
get: (target, name) => {
if (name !== PROXY_REST) {
used.current[name] = true;
}
return target[name];
},
};
const restHandler = {
get() {
return (excludingKeys) => {
const obj = this; // I didn't expect this works. Is it stable??
const clone = {};
Object.keys(obj).forEach((k) => {
if (!excludingKeys.includes(k)) {
clone[k] = obj[k];
}
});
return createProxy(clone);
};
},
};
const createProxy = (obj) => {
const proxy = new Proxy(obj, handler);
Object.defineProperty(proxy, PROXY_REST, restHandler);
return proxy;
};
const s = { a: 1, b: 2, c: 3, d: 4 };
const p = createProxy(s);
// const { a, b, ...rest } = p;
// ...transpile the above to the below by babel or by hand.
const { a, b } = p; const rest = p[PROXY_REST](['a', 'b']);
console.log(a, b, rest.c);
console.log(used);
I was thinking about something like
const { a, b, ...rest } = p;
// ...transpile the above to the below by babel or by hand.
const { a, b } = p;
const rest = WITHOUT_TRACKING(p, () =>
const { a, b, ...rest } = p;
return rest;
});
Your variant is a bit more compact, but I am not sure which one is easier create using babel. @faceyspacey - have you started anything?
@ScriptedAlchemy has successfully implemented this in Babel:
//INPUT:
export const MyComponent = (props, state, actions) => {
return <div>{state.title}</div>
}
// --> OUTPUT:
// rename the original component
const MyComponentOriginal = (props, state, actions) => {
return <div>{state.title}</div>
}
// and then simply use it as a function within the template:
export const MyComponent = (props) => {
const { state, actions } useRespond('__respond__currentFileName') // for our "Redux Modules" capability
return MyComponentOriginal(props, state, actions)
}
useRespond
usesuseReduxState
anduseReduxDispatch
under the hood
https://github.com/faceyspacey/remixx/blob/master/src/babel/helpers.js#L84-L94
We are trying not to touch the work ofreact-hooks-easy-redux
, and see how far we can go with our tasks on the babel side and the work of this lib as a dependency.
We got a bit more work at this stage, but we are going to attempt the useMemo
+ babel approach that @theKashey recommended. This is the approach where we detect what keys on state
are used (and if they are selectors) in order to pass as dependencies to useMemo
. Then of course we gotta fork Redux and allow for passing in selectors, plus the work related to namespacing for "redux modules."
That is to say, we got our hands full, but we'll take the assignment of selectors. We are hoping to depend on this library for good perf/benchmarks + rest operator. And we will give @dai-shi credit and mad "props" everywhere we go as we promote our tools. @dai-shi perhaps you want to join our unofficial team of collaborators. We are a rag tag decentralized army of gorilla warfare warriors, combatting the status quo ๐
t-shirt, or at least stickers to share?
t shirts maybe, stickers are in the works
const rest = WITHOUT_TRACKING(p, () =>
const { a, b, ...rest } = p;
return rest;
});
Does this help? We want to track rest
afterward, right? @theKashey
you want to join our unofficial team of collaborators.
Sure. Stickers!
Does this help? We want to track rest afterward, right? @theKashey
You will get the proxy tracker as a rest
, but you will not track that "get".
console.log(a, b, rest.c);
I assumed that affected would be ['a', 'b', 'c']
in this case. And it is so in my example.
The same should be for mine - tracking is off only when you are extracting rest
from the source object - later it's on again.
You approach would not work for one (edge) case - nested proxies, as long as it still would read from underlaying proxy. My approach just killing tracking, at all, giving you do that every you wanna to, and turning it back. I've used another syntax just to make babel plugin simper (just wrap). Read it like
const REST = (source, excludingKeys) => {
const clone = {};
Object.keys(obj).forEach((k) => {
if (!excludingKeys.includes(k)) {
// reading from proxy!
clone[k] = obj[k]; // would keep refs to the underlaying proxy-objects
}
});
return createProxy(clone);
};
const { a, b } = p;
DISABLE_ALL_TRACKING(); const rest = REST(p, ['a', 'b']); ENABLE_ALL_TRACKING();
const obj = this; // I didn't expect this works. Is it stable??
I wasn't sure if this is correct. Maybe, I need to try nested proxies. Will try.
DISABLE_ALL_TRACKING(); const rest = REST(p, ['a', 'b']); ENABLE_ALL_TRACKING();
Yes, this one should work. I'd explore how I could eliminate killing tracking, because I expect some use cases without babel (writing directly by hand).
$ npx babel-node
> const { proxyState, REST } = require('./src')
undefined
> const state = { a: { a1: 1, a2: 2 }, b: { b1: 1, b2: 2 }, c: { c1: 1, c2: 2 } }
undefined
> const trapped = proxyState(state)
undefined
> trapped.state.a.a1
1
> trapped.affected
[ '.a', '.a.a1' ]
> const rest = trapped.state[REST](['a'])
undefined
> trapped.affected
[ '.a', '.a.a1' ]
> rest.c.c1
1
> trapped.affected
[ '.a', '.a.a1', '.c', '.c.c1' ]
This is what I expect, and it seems working.
I spend so much time fighting spread last year. It's so simple to fight it with babel :(
(not super sure that TS could keep it untranspiled )
(not super sure that TS could keep it untranspiled )
@theKashey using babel + hypothetical babel plugin to transpile TS would be fine, right?
Just a thought: can we assume the transpiled result, and do a hack?
The babel repl transpiles a rest operator into:
var a = state.a,
rest = _objectWithoutProperties(state, ["a"]);
So, what if we can override the implementation of _objectWithoutProperties
or detect the call of _objectWithoutProperties
and do something.
Hmm, I don't like this idea very much though. I want a solution a) that works without babel, and b) babel just makes it easy.
fyi, Respond Framework (the Remixx library specifically) is the perfect place for the babel plugin approach to the rest operator--because we already have one. Here's what we have so far (thanks to @ScriptedAlchemy): https://github.com/faceyspacey/remixx/blob/master/src/babel/index.js
@theKashey is u can make a spec for Zack to implement this, that will really help us.
@MrWolfZ 's comment: reduxjs/react-redux#1179 (comment)
there are some cases in which unnecessary renders are triggered due to intermediary values being required to produce the final derived value (e.g. the example @epeli posted above, conditional accesses like from @faceyspacey's comment, or something simple like
Object.keys(state.items)
)
Another simple case (which is rather common) and technically difficult one is the rest operator.
const { a, b, ...rest } = state.items;
const [first, second, ...rest] = state.items;
Of course, arrays, so we have to have two different rest operators.
Yep.
@dai-shi you added support for the first one with objects, but not with arrays [first, ...rest] ?
the current implementation would convert array to the object ๐คทโโ๏ธ
I think we should be able to take the same approach for array rest.
object rest:
rest = proxied[SYMBOL_OBJECT_REST](['a', 'b']);
array rest:
rest = proxied[SYMBOL_ARRAY_REST](2);
Yep.
This issue has been almost only about rest operator. If we need to discuss about spread operator, let's open a new issue.
The remaining is to add a note in README.
Is this correct? @theKashey
Advanced Usage
Rest operator
You can't use rest operator to track state usage properly. There is a workaround. Refer this example.
import React from 'react';
import { useReduxState } from 'reactive-react-redux';
import { proxyObjectRest, proxyArrayRest } from 'proxyequal';
const Person = () => {
const state = useReduxState();
// const { firstName, lastName, ...rest } = state; // this is not recommended for performance.
const [firstName, lastName, rest] = proxyObjectRest(state, ['firstName', 'lastName']); // this is workaround.
return (
<div>
<div>{firstName}{' '}{lastName}</div>
<PersonDetail person={rest} />
</div>
);
};
Technically the current implementation of rest is 99% implementation of spread, but I am not sure it is the best way.
Probably a small and specific proxy is better
const spread = {
...object1,
...object2,
}
// ->
const spread = proxyObjectSpread(object1, object2);
// ------
cosnt proxyObjectSpread = (...objects) => {
const assingIndex = (object, x) => Object.keys(object).reduce((acc, key) => {acc[key]=x;return acc;},{});
// create "shape" objects, with values === used used object id
const spreadKeyHolder = Object.assign({},...(objects.map(assingIndex));
return new Proxy(spreadKeyHolder, {
get(key) {
return objects[spreadKeyHolder[key]][key]; // redirect read
}
})
}
PS: You are 100% right.
The remaining is to add a note in README.
You know, now I am thinking about adding d.ts
for proxyequal, which is quite important for this case.
But I would prefer you if will re-export necessary APIs via your library, or hide everything behind babel-plugin (@ScriptedAlchemy, where are you?)
prefer you if will re-export necessary APIs via your library
That's something I'd like to hear. So, do you mean it's cleaner? Any other benefit of re-exporting?
hide everything behind babel-plugin
I'd like to see how it's done. Depending on it, I'd choose which to recommend.
So, do you mean it's cleaner? Any other benefit of re-exporting?
You shall not use 5 different libraries, if they are all "behind" one. Some eslint configurations will not let you import packages you are not explicitly depend on.
Got that point. I'm aware of the eslint rule.
Current status: useTrackedState doesn't support rest operators any more (since v3). The experimental useTrackedSelectors should still support it (the remaining is re-exporting).
For respond-framework, useTrackedSelectors should work better. Waiting for feedbak.
Let me close this because of inactivity.
As of now, this library doesn't explicitly support rest operators. (It works by design with false positives = extra re-renders.)