Originscope for Yield Tag

[octotree]

Hey guys,
after running into the yield scope trap last week (#1965), I tried to write a patch to solve the problem. It took me a few days to find out how riot works, but I think I found a solution which should not break anything existing.

1. Have you added test(s) for your patch? If not, why not?
   No, because I wanted to find out first, if you consider merging the feature at all (I do not know yet how to write test for riot). Meanwhile, I wrote a demo site to test the functionality.
2. Can you provide an example of your patch in use?
   http://plnkr.co/edit/YX8pV8h6ShwaCJGWvGR9?p=preview
   The difference in usage can be found in the following files:
   • container-test-block-unpatched-tag.html
   • container-test-block-patched-tag.html
3. Is this a breaking change?
   No, without the new originscope attribute, riot works as it did before.

Content

The problem at stake is that yield tags are currently evaluated in the child scope they are placed in. But since the evaluated expressions come from a different tag/component this is very unintuitive. Furthermore, it creates complicated, unmaintainable code.

To not break anything, the originscope attribute is introduced:

When this attribute is present, the yield tag will be replaced by a riot-yield tag which inherits all properties from the tag which holds the html, which is in turn the template for the riot-yield tag. This way the inheritance does not affect the tag which holds the yield tag. This does not only work for properties, but also for named tags.

So without the patch it is very hard to access named fields e.g. (container-test-block-unpatched-tag.html):

self.tags['basic-container-double'].tags['basic-container'].tags['basic-container']['input-three'].value = self.num

With the patch this becomes very simple (container-test-block-patched-tag.html):

self['input-three'].value = self.num

I think this simplifies the usage and improves the overall code-quality of the developed components.

For riot 3.0 I would suggest to replace the existing yield implementation with something which behaives more like the riot-yield tag.

Please take a look at the demo and tell me what you think :-)

[GianlucaGuarini]

@octotree first of all I would like to thank you for the time you took to:

1. patch riot adding a solution to one of our issues
2. reading and answering the questions we provide for the pull request template
3. for providing a clear description of the problem and a demo to show your patch

I have checked your code and honestly I am not convinced ( not because of your code or your implementation ). In this process of simplifying riot trying to release asap riot@3.0.0 we are removing stuff rather than adding new options or methods to the API. The problem that this patch tries to solve is flawed: I know many riot users build their apps in a really simple way but the idea of relying on the parent -> children relations to inherit your app data is really bad and the yield issues are just a side effect of it. Your app data should not be stored in your tags, riot tags are just the tool you use to change and display your app state (aka single direction flow)

I will try to be more clear. The yield tag was introduced to solve mainly situation where tag templates should integrate markup provided by serverside rendering (basically stuff you can not receive via json api...). I see many riot users abusing the yield tag to achieve stuff could do with normal riot components (90% of times with 3 lines of code) so I don't want to extend/maintain any new single line related to the riot yield feature. Of course that's just my opinion I would like to know also what the others think about this.

cc: @rsbondi @rogueg @aMarCruz @cognitom

[rogueg]

I agree that the current scoping of yield is confusing, and I'm all for changing it.

I think yield is a powerful tool, and we should work to make it better. Sure, it can sometimes be abused to create convoluted code...but other times it's the most readable solution to a problem. I use it occasionally (though never in the "serverside markup" scenario).

I like how short your implementation is, but I think adding riot-yield elements into the markup is undesirable. I wonder if it's possible to leave the DOM as is, and just change the scope that expressions are evaluated against.

[octotree]

@GianlucaGuarini thank your for reviewing the patch. I think I understand your problem with the existing code and I know that this patch is build upon the existing 2.6 infrastructure and therefore amplifies the existing problems.

On the other hand, I know that every good component library needs a tool to build general containers. A few common examples where you need them are layouts, grids, sliders, lists, etc. But I am not sure how to build them in riot.

I think the yield tag, as a marker where to place the content of the container, is a good idea. And multiple slots are also reasonable for some use cases. So we definitly should keep the feature somehow. But maybe we can build it without the whole inheritance stuff.

When I thought about this scoping topic, before I looked into the code, I thought it must be possible to evaluate the expressions within the originating tag and output the result at the position of the yield tag afterwards. Sadly I do not know how this would affect the whole update loop (caching issues?!?).

@rogueg I completly agree with you. I thought about building the patch without chaging the dom, but I did not want to risk causing side effects and therefore went with the save 'build a bubble for each yield' solution.

I really love riot for its simple API and every line I added while writing the patch hurt a little, because I knew it could increase the size of the lib. But on the other hand, I know that this is an important aspect if you want to use it to build reusable component hierarchies.

[GianlucaGuarini]

Thank you for this pull request but riot 3 is now feature complete see and we will focus only on riot 4 that will be a complete rewrite.
I am not willing to merge this PR. Thank you so much for all you efforts
✌️