Skip to content
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

incremental-dom support #2749

Closed
ianmstew opened this issue Sep 16, 2015 · 33 comments
Closed

incremental-dom support #2749

ianmstew opened this issue Sep 16, 2015 · 33 comments

Comments

@ianmstew
Copy link
Member

It's time to bring smart DOM updates handled by a diffing algorithm into Marionette's templating layer. incremental-dom is showing a powerful affinity with Backbone/Marionette's templating philosophies because one of its core objectives is mapping from string templates. There is absolutely no reason why View -> DOM data binding should be a hesitation point when it comes to choosing Marionette.js as a framework for new projects.

@jridgewell and @thejameskyle are hard at work on a JSX -> iDOM plugin, now it's up to us to start researching the combination, possibly as a core plugin.

This is something @marionettejs/marionette-core is deeply interested in. Let's kick off the discussion!

@ianmstew ianmstew added this to the v3.x milestone Sep 16, 2015
@MeoMix
Copy link
Member

MeoMix commented Sep 16, 2015

👏

I'm building a new website from scratch and evaluating all the tools out there currently. My heart is with Marionette and I have little-to-no desire to learn Angular/React/Ember for anything more than educational purposes. However, the most glaring pain point for me is how behind the times Marionette is on DOM rendering / data-binding.

I'm not sure what I can contribute to the discussion, but if I can help at all -- lemme know! If this is a high-priority item with a reasonable time-frame for implementation then I'll happily choose Marionette as the framework I build applications with. :) 🎈

@ahumphreys87
Copy link
Member

@MeoMix myself and @ianmstew want to look at a handlebars => idom compiler, if you are interested - more hands would be appreciated.Its very early in the process but we will be working here: https://github.com/ianmstew/handlebars-idom

@MeoMix
Copy link
Member

MeoMix commented Sep 16, 2015

@ahumphreys87 I googled around on this a bit and found the issue on Handlebars: handlebars-lang/handlebars.js#1080 which was closed as out-of-scope. I then found the issue on htmlbars, tildeio/htmlbars#405, which was basically closed as "We already do something close enough to this -- why bother?"

It left me wondering where handlebars-idom fell in all of that mixture.

@Tvrqvoise
Copy link

Is this a Marionette problem? AFAIK, all we need to do is enable async render, then we can solve this problem at the template level, or even by incorporating simple things like https://github.com/wilsonpage/fastdom into our renderers.

After that, we can solve the publicity problem by supporting glimmer / react / whatever as an out-of-the-box renderer option.

@jamiebuilds
Copy link
Member

The problem here is that you're never going to get the full benefit of templating like this unless initialization/rendering/cleanup becomes completely managed by the framework which is very against how Marionette is designed today. In fact if you tried to make Marionette do that you'd have virtually no reason to use Marionette, as everything from CollectionViews to LayoutViews and Regions to ui don't make sense anymore.

@MeoMix
Copy link
Member

MeoMix commented Oct 5, 2015

@thejameskyle Could you elaborate on why?

@jamiebuilds
Copy link
Member

So let's imagine a typical Marionette tree.

var App = Marionette.LayoutView.extend({
  template: function() {
    return `<aside></aside><main></main>`;
  },
  regions: { sidebar: 'aside', content: 'main' },
});

var Content = Marionette.ItemView.extend({
  ui: { links: 'a' },
  events: { 'click @ui.links': 'onClick' },
  onClick: function() { ...}
  template: function() { ... }
});

var SidebarLink = Marionette.ItemView.extend({
  template: function() { ... },
  ui: { link: 'a' },
  events: { 'click @ui.link': 'onClick' },
  onClick: function() { ...}
});

var Sidebar = Marionette.CollectionView.extend({
  childView: SidebarLink
});

var app = new App();
app.render();
app.content.show(new Content());
app.sidebar.show(new Sidebar());
app.$el.appendTo(document.body);

Here it is in idom template functions (using jsx to simplify things):

function onSidebarClick() { ... }
function sidebar() {
  return (
    <aside>
      {sidebarItems.forEach(item => {
        <a ref="link" onClick={onSidebarClick}>{item}</a>
      }}
    </aside>
  );
}

function content() {
  return <main>...</main>
}

function app() {
  return (
    <div>
      {sidebar()}
      {content()}
    </div>
  );
}

If you compare the entire API from Marionette, it's all built into the template layer in a iDom/React-like api. If you try to force the Marionette conventions over it, you are just making it more verbose and less declarative. Also, the rendering needs to be managed or idom templates are actually slower than regular string templates.

@MeoMix
Copy link
Member

MeoMix commented Oct 8, 2015

Interesting. I see your points.

What do you think the situation calls for? Personally, I do feel that Marionette's current interactions with the DOM feel antiquated. Does this mean Mn needs it's own type of "incremental dom" which is baked into the framework itself? Or is this a more inherent shortcoming of Mn which means it will always require a more manual manipulation of DOM state?

@jridgewell
Copy link
Contributor

The problem here is that you're never going to get the full benefit of templating like this unless initialization/rendering/cleanup becomes completely managed by the framework which is very against how Marionette is designed today.

I'm going to disagree here, but first I'm going to agree with what you said next:

In fact if you tried to make Marionette do that you'd have virtually no reason to use Marionette, as everything from CollectionViews to LayoutViews and Regions to ui don't make sense anymore.

Yes these virtual-DOM/component libraries provide a fundamentally different way to to build apps that eliminate a lot of what Backbone/Marionette do.

So back to the first comment. We don't need the full benefit here, just a better way to update the DOM than the current this.$el.html(...). That's just too damn slow to be useful. So most devs handwrite update logic so that the the whole view isn't re-rendered:

class View extends Backbone.View {
  ui() {
    return { attrSpan: "span.attr" };
  }

  initialize() {
    this.listenTo(this.model, 'change:attr', this.onChangeAttr);
  }

  onChangeAttr(model, value) {
    this.ui.attrSpan.text(value);
  }
}

If there are multiple attrs within the template, there end up being several of those functions. All of that is duplicated logic: the template already knows how to render the attrSpan element and everything else. My goal for iDOM is to make #render so damn fast that I never have to write another #onChangeAttr again. Instead, just listen to the change event, and re-render.

class View extends Backbone.View {
  initialize() {
    this.listenTo(this.model, 'change', this.render);
  }
}

Yah, that's the stuff. :high:

So no, we probably won't take advantage event listeners directly in the DOM or regions (though maybe placeholder support for regions, but just the tip). But, we'll use iDOM just like we use our current mustache templates and it'll make our apps better/faster/smaller.

@jamiebuilds
Copy link
Member

My goal for iDOM is to make #render so damn fast that I never have to write another #onChangeAttr again. Instead, just listen to the change event, and re-render.

If you use plain string-based templates they are actually generally faster than any virtual-dom implementation (although they generally use more memory). I don't see performance as a benefit at all.

The main benefits are:

  • uniform method for creating/updating dom.
  • non-destructive updates to dom.

However, as I mentioned, once you are authoring views this way, almost all of Marionette's functionality becomes useless and if forced in would just make rendering slower.

The only reason I could see justifying this is to help people who want to migrate from Marionette to a virtual-dom based framework.

@ianmstew
Copy link
Member Author

What I appreciate most about Marionette is its API--it's view lifecycle, it's simple DOM event aggregation model on the root element, and the ability to keep logic out of templates. Yes, CollectionView may no longer be necessary, but if all I need to build an app is a Marionette 3.0 View I'd be pretty happy.

Marionette as a thin app layer between a developer and iDOM is promising for all the reasons we like Marionette in the first place--it's a library, not a framework, and it enables the developer to use the patterns of his/her choosing. I don't think the danger is that Marionette can't flourish with a vdom engine, I think the larger issue is whether Backbone.Model is still the best data binding source in a vdom-based view layer, but that's a separate discussion.

As far as whether the templating engine handles view creation or not, I am very much interested in templates that accept React-like components while leaving component lifecycle management to the user, similar to the discussion here: jridgewell/babel-plugin-transform-incremental-dom#9 (comment). Marionette could leverage component rendering delegation into a new method for rendering child views.

My plan is to experiment and research what Mn + iDOM could look like. While I hear @thejameskyle's argument that this work is fruitless except possibly as an intermediate step to migrate away from Mn, I sincerely request that further discussion on this thread be framed on how to proceed not whether to proceed with this research. Regardless of the end result, the process itself will prove a valuable learning experience and could lead in a direction we have not yet envisioned.

@epikhighs
Copy link

I'd love to see support for idom in Marionette with the objective of allowing marionette apps to leverage unidirectional data flow without the penalty for inefficient DOM updates. Basically, I'm totally in favor of what @jridgewell had in his comment. On a model change, just render everything again.

Of course, this would fundamentally change Marionette APIs, but I think it would be for the better. State management is a difficult problem and unidirectional data flow seems like an elegant, simple solution.

There's a lot of react hype, but what I see from there is the story of vdom. And vdom is not react exclusive.

@ahumphreys87
Copy link
Member

@epikhighs I've actually been working on adding some idom views to Orchestra https://github.com/BedeGaming/orchestra, I haven't pushed anything yet but I have got ItemView and LayoutView working with incremental-Dom templates, works with sub views as well so you can still leverage child events et al.

I'm not sure about bringing incremental Dom into marionette core, it may be just too big an opinion to throw on the community, but my aim for orchestra is to provide an opinionated set of tools for marionette apps

@JSteunou
Copy link
Contributor

A Marionette plugin would be a good idea in order to keep Marionette not so much opinionated but open that iDOM door for Marionette users.

I too agree with what @jridgewell said, even if I reduced a lot that handwrite update logic to just using Backbone.Stickit so it does that update for me, but yeah, just calling render you would be damn cool.

@JSteunou
Copy link
Contributor

there was some work on vdom https://github.com/tiagorg/marionette-vdom and a thing on idom https://github.com/MarionetteLabs/handlebars-idom

@ahumphreys87
Copy link
Member

Yup my fork of handlebars-idom is more up to date, compiles the majority of
the handlebars syntax now. There's also a browserify transform. Once I have
orchestra working with it I'll create a sample app with comparisons

On Monday, 25 January 2016, Jérôme Steunou [email protected] wrote:

there was some work on vdom https://github.com/tiagorg/marionette-vdom
and a thing on idom https://github.com/MarionetteLabs/handlebars-idom


Reply to this email directly or view it on GitHub
#2749 (comment)
.

@JSteunou
Copy link
Contributor

Nice looking forward to it

Yup my fork of handlebars-idom is more up to date, compiles the majority of
the handlebars syntax now. There's also a browserify transform. Once I have
orchestra working with it I'll create a sample app with comparisons

On Monday, 25 January 2016, Jérôme Steunou [email protected]
wrote:

there was some work on vdom https://github.com/tiagorg/marionette-vdom
and a thing on idom https://github.com/MarionetteLabs/handlebars-idom


Reply to this email directly or view it on GitHub
<
#2749 (comment)

.


Reply to this email directly or view it on GitHub
#2749 (comment)
.

@rtibbles
Copy link

rtibbles commented Feb 9, 2016

@ahumphreys87 Interested to see your work on the Handlebars IDOM - we're interested in using it, is there anything me or my team could do to help it get production ready?

@ahumphreys87
Copy link
Member

@rtibbles the repo is here: https://github.com/ahumphreys87/handlebars-idom you can run the app which will compile example.hbs there is also a browserify transform located here: https://github.com/ahumphreys87/hbsidomify

Most of the handlebars syntax is compiling - ive used it in a sample app. Ive actually got some updates that will create elementPlaceholders based on an an attribute in the markup - these can then be used to nest views (similar to regions).

My sample app has an ItemView and LayoutView rendering using incremental-dom - I tested rendering a list of 1000 items and shuffling it every 10ms (only a few items changed) idom copes with this brilliantly giving me repaint rate of 100 per second. Default marionette struggles as it is changing the full view rather than just those that change

@MeoMix
Copy link
Member

MeoMix commented Feb 10, 2016

@ahumphreys87 Do you have a list of TODOs you'd like to see completed before considering a release candidate?

@ahumphreys87
Copy link
Member

@MeoMix I do not as of yet. The one key item is importing the test suite
from Handlebars - handlebars-idom has no tests :(

Once Mn 3.0 is out I will be investing more time in this. I think the thing
that will take Mn to the next level is the tooling around it, things like
this and an isomorphic plugin.

On Wed, Feb 10, 2016 at 5:22 PM, Sean Anderson [email protected]
wrote:

@ahumphreys87 https://github.com/ahumphreys87 Do you have a list of
TODOs you'd like to see completed before considering a release candidate?


Reply to this email directly or view it on GitHub
#2749 (comment)
.

@rtibbles
Copy link

Been having a look at this. I assume most of your ongoing work is internal to the compiler?

I ask because the easiest way to integrate the handlebars tests will be to create a Handlebars equivalent API using your compiler instead - if that seems reasonable, I'll work on getting that implemented so that we can run the full suite of Handlebars tests from handlebars-idom.

@ahumphreys87
Copy link
Member

Hey, that would be great - to tell the truth, I'm not sure how far away the
api is from the one in handlebars, it could be quite far removed.

Would be great if you want to get involved, it could be a lot of work given
the output is quite different to what handlebars will produce.

Maybe the handlebars tests aren't the best way to go, maybe it would be to
better to focus on end to end testing and test the output of many examples
are correct, I'm open to suggestions

On Thursday, March 10, 2016, Richard Tibbles [email protected]
wrote:

Been having a look at this. I assume most of your ongoing work is internal
to the compiler?

I ask because the easiest way to integrate the handlebars tests will be to
create a Handlebars equivalent API using your compiler instead - if that
seems reasonable, I'll work on getting that implemented so that we can run
the full suite of Handlebars tests from handlebars-idom.


Reply to this email directly or view it on GitHub
#2749 (comment)
.

@rtibbles
Copy link

Assuming I'm not misreading them, I think most of the Handlebars tests focus on compiling a template, and then evaluating a template and checking its output against what it should have evaluated to. Assuming I have read this correctly, this would seem a good approach to take in testing handlebars-idom also.

@ahumphreys87
Copy link
Member

If that is the case then yes, they sound ideal!

On Thursday, March 10, 2016, Richard Tibbles [email protected]
wrote:

Assuming I'm not misreading them, I think most of the Handlebars tests
focus on compiling a template, and then evaluating a template and checking
its output against what it should have evaluated to. Assuming I have read
this correctly, this would seem a good approach to take in testing
handlebars-idom also.


Reply to this email directly or view it on GitHub
#2749 (comment)
.

@atomictag
Copy link

In case anybody is still interested, I have just published incremental-bars, a library that allows to use incremental-dom with Handlebars templates. It is pretty trivial to integrate with existing frameworks like Backbone, Marionette etc.

You can check it out here. cheers

@JSteunou
Copy link
Contributor

@atomictag how does it compete with simple html string from handlebars to real dom diffed through diffing lib like morphdom or nanomorph

const node = createElement('div');
node.innerHtml = htmlStringFromHbs;
morph(oldEl, node.firstChild);

@atomictag
Copy link

I am not all that familiar with morphdom and nanomorph, but they both seem pretty great. There's some interesting discussion here on the topic.

incremental-dom only works when it is used as a compilation target by a template engine.
morphdom will work with any templating engine that produces an HTML string.

So if your input is always a string, morphdom and nanomorph seem to be perfect options.
In that respect they are very flexible as they don't depend on the template engine doing anything special. I have however a hard time imagining that a diff between a string and DOM at runtime can be faster than using an already tokenized input (because of the parsing and internalization that has to occur before the input can be compared with the current state) but only a benchmark could tell.

incremental-bars is strictly only applicable to handlebars and best suited to precompiled templates, for which a build-time compilation into a lower level target such as incremental-dom makes sense as it reduces what the browser has to do at runtime. At the end of the day it's good to have choices.

@JSteunou
Copy link
Contributor

I have however a hard time imagining that a diff between a string and DOM at runtime can be faster than using an already tokenized input

Sure, that's why I transform String to DOM through an temporary div through innerHTML.

@atomictag
Copy link

incremental-dom optimizes for memory, while still being pretty fast at rendering/patching nodes. Converting strings to html - even when done by the browser - I guess has 2 main memory overheads (significant only for large html chunks or very frequent re-renders): the string generated by the template engine and the dom tree created by the temporary div used for the internalization of the string. I presume it is also unavoidable to create new objects as the new input dom is walked to perform the diff - something that incremental-dom is very careful not to do (to lower the GC pressure). I found this comparison interesting, although my own independent tests did not confirm that the layouting / painting using idom is half as bad as shown here (so I don't necessarily believe this is a good benchmark to compare things).

@JSteunou
Copy link
Contributor

Thanks for the highlights, I will check more on idom :)

@paulfalgout
Copy link
Member

@blikblum we might be able to support this now with the dom api and set renderer.. Or does this need #3427 ?

@paulfalgout paulfalgout modified the milestones: v3.x, v4.x Aug 26, 2017
@blikblum
Copy link
Collaborator

#3247 is just for showing child views optimally. In fact in my current project i am using incremental dom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests