Yield slot not working when nested #1458

Closed
sntran opened this Issue Dec 18, 2015 · 13 comments

Projects

None yet

8 participants

@sntran
sntran commented Dec 18, 2015

Follow up from #1218, does this new feature expect the <yield to=""> as immediate children of the tag?

The following does not work (also see http://embed.plnkr.co/v5miAuEsD3FHxjgn33L1/)

Tag definition

<better-select>
  <yield from="options"></yield>
  <yield from="toggle"></yield>

</better-select>

Tag mount

<better-select>
  <yield to="options">
    <ul>
      <li>Option 1</li>
      <li>Option 2</li>
    </ul>
  </yield>
  <div>
    <yield to="toggle"><span class="icon-down-dir"></span></yield>
  </div>

</better-select>

Expected

<better-select>
  <ul>
    <li>Option 1</li>
    <li>Option 2</li>
  </ul>
  <span class="icon-down-dir"></span>
</better-select>

Actual

<better-select>
  <ul>
    <li>Option 1</li>
    <li>Option 2</li>
  </ul>
  <yield from="toggle" ></yeild>
</better-select>
@GianlucaGuarini GianlucaGuarini added the bug label Dec 18, 2015
@nippur72 nippur72 added a commit to nippur72/riot that referenced this issue Dec 21, 2015
@nippur72 nippur72 Fixes #1458 yield with nested tags 56ea0b2
@nippur72
Contributor

I've fixed the bug, mostly due to using querySelector and so not getting nested yield from.

I've rewritten the logic, now it loops over "yield from" nodes picking back the corresponding "yield to" node.

  • If there are more than one "to" nodes pointing to the same "from", the first will be used
  • if a "from" node does not have a corresponding "to" its content is cleared
  • if a "to" node does not have a corresponding "from" it stays in the dom as <yield to=".."> (it's unchanged)
@GianlucaGuarini
Member

@aMarCruz probably this feature should be handled using regex instead of relying on document.querySelector ( not supported serverside ) what do you think?

@aMarCruz
Member

@GianlucaGuarini , sure it is possible, I will see that it can be.

@aMarCruz
Member

@GianlucaGuarini , I misunderstood this feature, it is a bit complex and I guess can interfere with the fix for the if activation.
Anyway, I'm working on it. Just for clarify:

  • There's an one2many relation, the parent is the html element (the first yield with "to").
  • Unused "to" are left in the DOM (they will be overridden on mount).
  • "from" with no "to" are removed.
  • No expressions in the values of "from/to", only names in the ASCII range [-\w].
  • No nested <yield> tags.
  • None <yield> can have additional attributes.
  • Source yield ("to") is always closed with </yield>

right?

EDIT: We can have simple <yield> mixed with <yield from> ?

@GianlucaGuarini
Member

There's an one2many relation, the parent is the html element (the first yield with "to").

Yes exactly all the yield from having the same id will receive html from the single yield to

Unused "to" are left in the DOM (they will be overridden on mount).

yes they will be wiped from the mount event

"from" with no "to" are removed.

yes it will be easier to understand what goes wrong for debugging

No expressions in the values of "from/to", only names in the ASCII range [-\w].

yes

No nested tags.

yes

None can have additional attributes.

yes

Source yield ("to") is always closed with

yes

We can have simple mixed with ?

yes <yield from="foo"> | <yield from="foo"></yield> | <yield from="foo" /> should be all valid

@aMarCruz
Member

@GianlucaGuarini , and no nested <yield> <yield></yield> </yield> for sure. Thanks.

@lmicra
Contributor
lmicra commented Jan 3, 2016

Perhaps I can explain. <yield to=""> is expected as immediate children of the tag. Think of this as passing html to your tag options. And using your tag to place the html in the correct slots. The example shown on the first post is actually switched. Your tag definition is responsible for the presentation. When you insert your tag in the html it should be used as a component with options (some of which might be html).

@aminry
aminry commented Jan 15, 2016

I just upgraded from version 2.3.12 to 2.3.13 which has this fixed.
Now if I use Yield inside a loop it throws exception in case it needs to remove it (unmount it).
Here is my component.

<if>
<virtual each="{((opts.condition) ? [1]: [])}">
    <yield/>
  </virtual>
</if>

if opts.condition turns from true to false, when riot want to remove the node it throws following exception:

Uncaught TypeError: Cannot read property 'removeChild' of null

Here where the exception is happening:

  defineProperty(this, 'unmount', function(keepRootTag) {
    var el = root,
      p = el.parentNode,
      ptag

    self.trigger('before-unmount')

    // remove this tag instance from the global virtualDom variable
    __virtualDom.splice(__virtualDom.indexOf(self), 1)

    if (this._virts) {
      each(this._virts, function(v) {
        v.parentNode.removeChild(v)   <----------------------------------------------
      })
    }
@aMarCruz
Member

@aminry , this is not related to yield.
The expression for each must be in the format { [value [, index] in ] items } (regex /\{\s*([$\w]+)(?:\s*,\s*(\S+))?\s+in\s+(\S.*)\s*}/), so {((opts.condition) ? [1]: [])}is not a valid expression for each.

See this jsbin

@aminry
aminry commented Jan 15, 2016

I am not sure why {((opts.condition) ? [1]: [])} is not a valid expression, since it will result in [] or [1] which both are arrays, and based on the format you have mentioned it is supported.

I changed the code to following to make it more clear and still same issue exist, as soon as I assign empty array [] and the loop needs to remove the items, it throws error, (Again, this only happens on version 2.3.13), everything works just as expected on 2.3.12.
Besides this only happens when I use Yield tag in a loop, everything else works just fine.
I debugged the code, and it seems unmount is called twice, the first time is successful, and the second time it fails, because there is nothing to remove.

<if>
  <virtual each={ data }>
    <yield/>
  </virtual>
  <script>
    this.data = [];
    this.on("update", () => {
      if (this.opts.condition) {
        this.data = [1];
      } else {
        this.data = [];
      }
    });
   </script>
</if>
@aMarCruz
Member

@aminry thanks, seems related to virtual tags, maybe @rsbondi can help us.

@rsbondi
Contributor
rsbondi commented Jan 15, 2016

I think the same issue as #1512

@paulmarrington

I have a problem and this fix has negated my fix. I hope my example below is clear. I have a code-editor component and I want to define the contents of a button bar. The code-editor is built with an inner panel element that displays the button bar. So, I need to pass a yield to an inner component for rendering. This used to work: <yield to=buttons><yield from=buttons/></yield>, but no longer. Any suggestions on a work-around?

<my-component>
  <code-editor ...>
    <yield to=buttons>..button html...</yield>
  </code-editor>
</my-component>
...
<code-editor>
  <panel ...>
    <yield to=buttons><yield from=buttons/></yield>
  </panel>
</code-editor>
...
<panel ...>
  <div ...><yield from=buttons/></div>...
</panel>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment