Skip to content

Add initial support for {{yield}} #86

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

Closed
wants to merge 2 commits into from
Closed

Add initial support for {{yield}} #86

wants to merge 2 commits into from

Conversation

FWeinb
Copy link
Contributor

@FWeinb FWeinb commented Dec 1, 2016

I implement {{yield}} just as described in #5. To write tests I had to pass the assert library in the compileError method on the config object like it is done for the test method. Hope this goes in the right direction.

I don't think that every use case is solved by this but I think this is a good starting point.

@codecov-io
Copy link

Current coverage is 90.76% (diff: 100%)

Merging #86 into master will increase coverage by 0.03%

@@             master        #86   diff @@
==========================================
  Files            48         48          
  Lines          1305       1310     +5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1184       1189     +5   
  Misses          121        121          
  Partials          0          0          

Powered by Codecov. Last update 4f6a49b...57ae613

@Rich-Harris
Copy link
Member

Thanks for having a stab at this. I think it's a little trickier, sadly – the {{yield}} block could be inside an {{#if ...}} block, for example – so what we'll probably need to do is supply a function that renders the yield fragment. Maybe something like this (this assumes we end up separating mount from instantiation as in #91):

  function renderMainFragment ( root, component ) {
    var main = document.createElement( 'main' );

    var widget_yieldFragment = renderWidgetYieldFragment( root, component );

    var widget_initialData = {
      foo: root.foo
    };

    var widget = new template.components.Widget({
      target: main,
      parent: component,
      data: widget_initialData,
      yield: widget_yieldFragment
    });

    return {
      mount: function ( target, anchor ) {
        target.insertBefore( main, anchor )
      },
    
      update: function ( changed, root ) {
        var widget_changes = {};

        if ( 'foo' in changed ) widget_changes.foo = root.foo;

        if ( Object.keys( widget_changes ).length ) widget.set( widget_changes );
        
        widget_yieldFragment.update( changed, root );
      },

      teardown: function ( detach ) {
        if ( detach ) main.parentNode.removeChild( main );

        widget.teardown( false );
      }
    };
  }

  function renderWidgetYieldFragment ( root, component ) {
    var text = document.createTextNode( "Hello " );
    
    var text1 = document.createTextNode( root.a );

    return {
      mount: function ( target, anchor ) {
        target.insertBefore( text, anchor );
        target.insertBefore( text1, anchor );
      },

      update: function ( changed, root ) {
        text1.data = root.a;
      },

      teardown: function ( detach ) {
        if ( detach ) {
          text.parentNode.removeChild( text );
          text1.parentNode.removeChild( text1 );
        }
      }
    }
  }

So, the widget would use the yield fragment's mount and teardown methods, but the component that contained the widget would use its update method.

There might also be some nuance here around what happens with event handlers etc...

@FWeinb
Copy link
Contributor Author

FWeinb commented Dec 2, 2016

Thanks for your great writeup. I just tried to get the most basic {{yield}} working. I think it will be necessary to evolve this into a much more versatile solution before it will be usable.

I think the first thing would be to develop #91 and come up with the whole picture of what the resulting javascript should look like. Maybe even think about a way to have some common patterns written to a standalone library (optionally) to make it possible to compile applications without some boilerplate.

Nevertheless In my opinion you are on to something here. This is definitely moving in the right direction for web development pushing the framework into the compiler!

@FWeinb
Copy link
Contributor Author

FWeinb commented Dec 4, 2016

Closing in favour of #112.

@FWeinb FWeinb closed this Dec 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants