-
Notifications
You must be signed in to change notification settings - Fork 51
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
Documentation improvements and questions #95
Comments
Regarding this.state in render function, your problem is probably related to function binding. When you use "lambda" style functions with e.g. React's preprocessor, the function is automatically bound to the the current component((props) => {
...
});
// will be transformed to
component((function (props) {
...
}).bind(this)); That is why none of the component members are available when you use that style, because it is most likely bound to the window context. |
Hi, thank you, Adam! Anyway it would be nice to stress this in the documentation. If "this" is not available, what workaround to use to access state/mixin methods? |
"this" will be automatically bound to the React component as long as you don't use lambda syntax for your render function :) component((props) => {
console.log(this); // => most likely "window"
console.log(this.state); // undefined
});
component(function (props) {
console.log(this); // => ReactComponent
console.log(this.state); // Your component's state
}); |
Hi, @jakubholynet and thanks for your feedback!
The README is ment as a summary / quick guide to what you need, and the homepage is more in-depth. I agree that some of the info on the README should also be mentioned on the homepage.
As @adamschoenemann is pointing out here, this is to do with ES2015, and is maybe why a lot of people disagree with the syntactic sugar that is fat arrows. I don't feel Omniscient.js' documentation should be so verbose as to document JavaScript features/gotchas. In most cases, you can use fat-arrows as syntactic sugar to components as in most cases var MyComponent = component(function (props) {
return <div>{this.state.message}</div>;
}); Regarding
Essentially, you can pass in any object literal as props, with support for cursors and immutable data. For instance, this can be done: React.render(MyComp({
cursor1: someCursor,
cursor2: someOtherCursor,
otherProperty: {
myImmutableData: Immutable.fromJS({ hello: 'World' })
}
}), document.body); But should this be done? Probably not. As with normal functions, you wouldn't your parameter set to be too large, right? Something like
As with all tools, this is in no way a silver bullet. Not every application is the best way to model in this way. For instance, the Atom.io editor just transformed to not use Reacts re-render model as it caused too many repaints. The same drawbacks of React will also, most likely, be apparent with Omniscient.js - all though, Omniscient would in most cases reduce some GC work and the render functions will be invoked less. That is another point, when the re-render interval is very high, and the virtual DOM is created many times, the GC might have a bad time clearing all of the objects created, thus the memory and CPU usage could get high. Thanks for your thoughts. I hope my comments help some. I've also updated #56 to have check points for your feedback. |
Thanks! Ad Ad local state) I have read the essey and understand the reasons. Yet there are cases when state is valuable. YMMV :) Ad multiple cursors) Yes, everything can be overused. Still I think it would be valuable to mention this, even if passing multiple cursors is normally not encouraged. But you always encounter special cases ... Ad limitations) I was actually asking about the specific limiations/prefered use cases of Omniscient wrt. other React libraries. Or is Omnicient the absolutely best React library for any case? But I can understand if you do not have yet enough field reports to answer that well. Thank you! |
Hei, I have some proposals/questions bt could not find any e-mail group or similar to discuss them first so I am just creating an issue.
1. Differences between https://github.com/omniscientjs/omniscient and http://omniscientjs.github.io/
Currently the README at github contains some stuff not on the web page, e.g. the Local State section. That makes it harder to find stuff, it would be better if they were in sync.
2. Local State (vs. props)
this.setState
must be called from a mixin, not diectly from the component's (implicit) render method, right?this.state
available inside the render method? It seems that when I usefunction
as incomponent([withInitialState], function(props) { ...}
then I can accessthis.state
but if I instead use ES6 lambda as in:component([withInitialState], (props) => { ...}
thenthis.state
is undefined. This discrepancy should be documented or preferably prevented. Or is the plan that the state should be read/written only in a mixin? I.e. I'd need to add agetState
to my mixin to access the state?swapProps
mixin instead of local state, right? It would be nice to explain why that is better in your view.Multiple cursors
I believe I have seen in an example a component that depends on multiple cursors, something like
Is that right? It would be nice to mention this possibility (and use case for it / how not to misues it) on the main page. It is obvious to you but to a newcomer like me, based on the examples, it seems that only a single cursor is typically passed in.
Limitations / when not to use?
Nothing is a silver bullet. It would be very useful if you described where you think Omniscient is a good fit and especially where it is not a good fit - to help me choose the right one of the many different React libraries. It would be helpful to list also the limitations / weaknesses of this approach, there are certainly some? I remember David Nolen describing issues with Om and some of that perhaps applies here as well (sadly, I do not remember the details and haven't the link; some of it was related to the inherently tree structure of the data, I believe).
Thank you!
PS: You can reach me via jakub (at) iterate.no if you want
The text was updated successfully, but these errors were encountered: