ReactJS feedback about browserhtml

[andreasgal]

I love this. Practical applied feedback. Tried it early. Didn't work great. Undo. Try something else or try again later. This is priceless learning experience.

Sent from Mobile.

On Jan 6, 2015, at 18:56, Paul Rouget [email protected] wrote:

I backedout React integration.

I played with it today. This new code base feels over engineered for such a simple UI (4 buttons, 2 inputs, tabs), and we still don't know what the UI will look like in the future.

I am not ruling out React, but it's a premature optimization. Let's reconsider using React later.

—
Reply to this email directly or view it on GitHub.

[Gozala]

It would have being premature optimization if intent of using react was optimization. While indeed React is bundled with a lot of optimizations that was not the main point (not to me anyway).

I've listed bunch of reasons in original pull request paulrouget/firefox.html#94

In my experience (specially at Mozilla) simple UIs quickly grow into complex ones and code quickly becomes unmaintainable hairball of state distributed across different components that are unnecessarily coupled. Usually that means making any change without breaking anything else is quite difficult.

The reason I wanted to integrate React early on was to avoid same route. React does provide & enforces very clear separation of concerns between components & localizes state into single place. This combined with unidirectional data flow model it encourages makes it a lot easier to reason about code & make changes without breaking unrelated parts. It is true that benefits become more apparent as code grows, but also does cost of switching (it already took me few days month from now it will be too much work to be worth it).

[paulrouget]

In my experience (specially at Mozilla) simple UIs quickly grow into complex ones and code quickly becomes unmaintainable hairball of state distributed across different components that are unnecessarily coupled

I agree. So let's make sure it doesn't happen.

I'm absolutely certain that the current code will be rewritten many times. There's no value in the current code base. It's just a sandbox.

We will revisit that later.

[paulrouget]

@Gozala, can you explain virtual-dom.js, links.js, component.js and element.js?

[Gozala]

@paulrouget

• virtual-dom.js Essentially it's just exports React.DOM as html, which is used in render methods to describe the node associated with a component. React takes care of DOM updates for you. In terms of API I think it's pretty straight forward html.div({ className: "bla", id: "foo" }, children).

• element.js In an original react integration branch I have mentioned that react does supports only subset of HTML nodes & attributes. Element function exported by that module let's you define custom node (tagName is first argument) and with extended list of attributes & events that are described in a second argument.

  Let's take a look at an example from code base

  Since mozbrowser iframes have bunch of non standard features those are not supported by react. There for instead of using html.iframe we are defining and using our custom IFrame. First three attributes remote, mozbrowser, mozallowfullscreen are special since need to be set before node is in the document to have an effect, such attributes are declare via Option. For example browser: Option("mozbrowser") means that options.browser from IFrame(options) will be set as an attribute mozbrowser on associated node before it is added to a document. The Attribute is used to declare regular attributes that can be updated even after node is in a tree, for example flex: Attribute("flex") just means that option.flex will be set as a value of flex attribute on the node. There is also focused: Field((node, c, p) => if (c) c.focus()) style definitions, which is a way to handle option.focused state changes when it does not translates to an attribute but rather to a function call(s). Finally onLoadStart: Event("mozbrowserloadstart") is the way to declare event handlers, in this case it means that option.onLoadStart will be used as an event handler for mozbrowserloadstart events.

• link.js For whatever reason react does not handles onLoad event on html.link so this is just our custom link tag definition that we use instead of html.link that is why it declares that option.onLoad will be used to handle "load" events.

• component.js is simply a factory function that generates React class and then factory out of it since users really only use factories. In addition it exposes only subset of API, idea was that this way will be able to swap out react with vtree + vdom instead without having to rewrite any of the component code we've written.

[paulrouget]

Alright. So I took some time to understand the code (thanks for the explanations).

My initial feeling is, if me, a not too bad JS developer, have a hard time understanding what's going on in this code, that means the code is too complex for many people.

Can we go for simple React and see where that brings us? Also - we don't have to use react everywhere. Like, the iframes can just be some regular HTML.

[paulrouget]

1. React has bunch of limitations (some where mentioned earlier in this thread) few abstractions here are specially created to workaround them.
2. Limitations of react do get into our way & react proved itself to be inflexible with non standard or new html stuff (like web components). For this reason we would like to keep react swappable for example with something like vtree + vdom. Use of abstraction will let us swap out react with something else without having to rewrite anything but an abstraction layer.
3. React does encourage no local state, but it does not enforces it hard enough. Introduced abstraction exposes just a good part of react enforcing that further.
4. React can not run in a web worker, while vtree + vdom can so keeping abstraction over these two options is good as it would allow us to experiment with moving some parts of code into web worker.

I understand most of that. We can address these points later as we get familiar with React.

[bbondy]

@paulrouget what's your stance on JSX?

[bbondy]

In particular you can write React w/ or w/o JSX. If you go with the code is easier to read and more resistant to changes as React develops as a library. But if you go with you also must transcompile your sources using an ES6 compatible fs watcher tool like jsx. I weighed in at a previous bug that I'd support using it, since as you said, you don't need to use it everywhere. You'd just require("build/tabs.js") instead of require("js/tabs.js") for example. But I won't push one way or the other other than stating my preference and asking for your preference.

[davidascher]

Drive-by from the peanut gallery. The Webmaker team has been exploring React as well (and we've decided to standardize on it for our webapps, FWIW, including JSX). It's early, but so far I like React quite a bit (esp. because of its performance and fit with mobile runtimes), however I have no opinion on its applicability to browser.html's specific scenario.

I figured I'd point out these two blog posts by Mozillians evaluating it in their own contexts:

@jlongster: http://jlongster.com/Presenting-The-Most-Over-Engineered-Blog-Ever
@Pomax: http://pomax.github.io/#gh-weblog-1419289380022 and https://pomax.github.io/#gh-weblog-1420592591221

And as an aside, @dmose was telling me at mozlandia that the Loop team's been using it as well, and his advice was to do full React+Flux as Flux helps manage complexity as apps grow more complex. /cc @thisandagain @simonwex

[dmose]

Yeah, we've been using it for Hello, and I think @Standard8 may have a blog post in the works with more details. My point of view about our use of it there (I also don't have sufficient context to speak to your use case) is that while it feels a bit odd to get started with, the immutable state in React combined with Flux pattern has yielded substantial benefits in bounding complexity and making things easier to reason about.

[paulrouget]

We now use React.