-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Consider Using JsonML as Notation for ReactElement #2932
Comments
cc @chenglou |
Oh boy! Gets excited anyways Here are some counterarguments, plus some other arguments on the pros of this.
I don't think we have to stick with JsonML's specs too closely, if ours make more sense. For example, why is its style value a string? Why not an object like in React? Maybe you can influence the specs? Now for the pros:
<Select><Option selected={true} /><Option /></Select> This is weird because we're passing a prop to a child, then making the parent read into it to determine some stuff. The theoretically better way: <Select selected={0}><Option /><Option /></Select> But this is hard to manage and might get out of sync. Even better way (@jordan): <Select items={[['Option', {selected: 0}], ['Option']]} /> In which case we have a good excuse of being able to read ['Select', {}, ['Option', {selected: 0}], ['Option']] Also easier for libraries like
|
I see this use case very often in my code. I usually hesitate between keeping stuff as data (and array prop) or using |
Yep. Not saying it's not possible with I think that we should work with vanilla JS collections as much as possible (including their generation), and keep React at the edge only. |
Oh and @sebmarkbage: children comparison becomes easier. You'll also get whatever optimization you want from Flow for free (e.g. pull immutable arrays outside to avoid allocations). There are probably a lot more [insert optimizations for normal arrays/tuples] that I haven't thought about. |
If this can be worked out from a performance standpoint, +1. If we can get some convergence from other libraries by a standard or even shared conventions +1,000. This does also make things like ReactRouter's use of JSX for describing routes more sane, and not tied explicitly to React.
Did you mean to have the quotes around Select and Option, or are those supposed to be references? |
No. Not sure why I quoted those. |
Okay, thanks. Also for the sake of clarity and compat I think putting any metadata last makes the most sense, like an optional parameter to a function should usually go last. ['div', props, children, {key: 1, ref: bla}] This makes handling this kind of data easier because you know (edited: sorry, accidentally hit the submit key shortcut while typing) |
@sebmarkbage It seems to me that the current object-based representation should be preferable from a technical perspective... in addition to actually making sense if you try to operate on them i.e. Something to consider is supporting JsonML via run-time/static transformation just like JSX instead, except it would be JS-compatible so there shouldn't be any tooling issues for static transformation. Wrap all React JsonML in render() {
return ReactJML(
['div', { className: 'container' },
[FancyButton, { disabled: true }],
['span']
]
);
} JsonML seems like yet another DSL for something which should have it's own dedicated syntax. I don't think the solution to problems like this is inventing ways of making the code less ugly with magical arrays (just because arrays have minimal character overhead), it's understanding that view hierarchies require a syntax designed for it (that's why we have HTML to begin with) and it's time for languages to catch up. |
I've done some tooling with jsonml in the past. I've considered jsonml to be [tagName, properties, children] with some special casing for text nodes.
I like the second library because it's a bunch of form helpers that are completely framework agnostic. All I've implemented for jsonml is creating elements & html strings. zero diffing or patching. I've since moved away from jsonml for a few reasons:
|
@syranide @Raynos good to know these exist. What do you mean by the perf cost? Can immutable collections mitigate this? |
@chenglou Still not a nice way to do it, you wouldn't do it like that if it wasn't for JsonML. My point is that it seems weird to make JsonML the target, why not make JsonML compile to the best target instead? Just like JSX does. |
❤️ |
@syranide yeah that's what clojurescript people do: https://github.com/reagent-project/reagent#examples |
@chenglou the internal representation of a virtual dom in both React & virtual-dom is not JsonML. This means you have to convert data structures, this includes a full deep copy basically. In virtual-dom we avoid this with a |
I just want to chime in with my 2 cents, and my experience with using react with coffeescript (and no jsx). At the top of my coffee file, I import in the elements I plan on using:
My render methods end up looking like:
Which is suprisingly similar to JsonML, but with less clutter (at least in my opinion). With this in mind, if this is only a readability issue, I think it's unnecessary to add another notation, since coffeescript enforces indentation which them clearly demonstrates parent/child relationships of elements. I've never had to use JSX for any of my components, so JsonML is not necessary in terms of an alternative to JSX. If JsonML is a necessity, it would probably be possible to just wrap that data coming in via native JS:
I don't know what sort of performance hit this actual has |
There may be good reasons that it wouldn't work, but along these lines, what if we used strings instead of references for child components? render() {
return ['div', { className: 'container' }, ['FancyButton', { disabled: true }], ['span']]
} This would obviate the need for shallowRender, and it would invert control for the components. You'd have to register your components centrally/pass them to |
I don't know about React's implementation in particular, but I've created a library that uses JsonML diffing (no JSX) with minimal hard syntax: https://github.com/danristea/DOMatic/tree/master |
I use JsonML a lot in my flow. Anybody know good react-to-jsonml custom renderier? |
@paranoidjk I need exactly the opposite tool. That would take React VirtualDOM structure and render it into JsonML like object or object proxy. Just custom implementation of ReactDOM.render. |
Seems like changing this would not solve any practical problems we’re experiencing, but would introduce new ones. |
I would actually solve 2 problems. It would remove two levels of indirection when creating AST.
If JSON-ML was supported natively you would not need to have those two steps, and would allow people who prefer JS over XML, to write templates directly in JS: const Button = () => ['button', {}, 'Click me!']; This also eliminates the Also, if template is "simple", i.e. not using complex props, like And, one of the biggest benefits would be that JSON-ML is standartized, so any tool that works with JSON-ML could pick it up and work with it. |
DON'T GET TOO EXCITED, this probably won't work. This issue is more about documenting why we can't do it.
We could consider using JsonML as the notation for ReactElement.
This would make this JSX...
...which currently looks like this...
...into a much nicer non-JSX declaration.
It would also align us with a spec which is not ours. Almost like a standard.
Children are Special
JsonML is designed in a way that children can be distinguished from other attributes. We don't really care about this property because we've already merged them, but we could undo that. NBD.
One problem is that you constantly need to slice off the children from the outer array as you're passing it into a component. This can become a performance problem.
key
andref
The problem with JsonML is that we don't have a way to attach custom attributes to the element, that are not props. I.e.
key
andref
. It is important that these are treated differently than other props because they should not be available to the component itself. Conceptually the parent is responsible for keying the element and changing the key should not affect the behavior of a component. Same for refs. It is also important that props can be transferred to a child without that affecting its behavior.One possible solution would be to wrap the elements in another object:
It's still ugly. It also means that you now have to reason about two types of element (with or without wrapper). These children are also ambiguous in the first position of an attribute-less element.
Constant Destructuring
Any time you need to clone or reason about an element you need to destructure the system.
Nested Arrays
We can't distinguish between a nested array of elements and just an element. This would only work if we enforced that children are always provided as flat lists and explicitly flattened as needed.
E.g.
['div', ...this.props.children, ['div', { className: 'extra-child' }]]
Flattening makes it very difficult to preserve keys though.
What About Allowing Both?
A component that receives a ReactElement from the outside should not need to reason about two different kinds of abstractions and use two different patterns for accessing the type or props. It is important that the React community doesn't diverge too far from each other.
The text was updated successfully, but these errors were encountered: