Talesoft / tale-jade

A complete and fully-functional implementation of the Jade template language for PHP
http://jade.talesoft.codes
MIT License
88 stars 10 forks source link

Nested mixin block content loses scope #98

Open jaydenseric opened 8 years ago

jaydenseric commented 8 years ago

In Pug, this:

mixin test
  block

- $text = 'Test text';

p=$text
+test
  p=$text

Renders:

<p>Test text</p>
<p>Test text</p>

In Tale Jade, the very same renders:


<p>
  Test text</p>

<p>
  </p>

This is a big blocker to porting my Pug templates over, not really sure how to handle it.

TorbenKoehn commented 8 years ago

Yeah, this is a real problem. Let me try to explain it.

When passing args to the Renderer, a variable called $__args gets filled with them and is put into the Jade template. This $__args variable is pretty important, since it's the variable that I use to pass the scope to functions/mixins (use ($__args), extract($__args))

Now, when you do - $text = 'Test text' inside the template, that one isn't added to $args dynamically anymore. It's also pretty hard to do (I'd need to diff get_defined_vars() after each code-block anywhere and put them in the `$args`-container.

What you can do to solve this:

Two methods.

I thought much about this, but it's just the way scoping works in PHP :(

jaydenseric commented 8 years ago

😱 So for the "refill" option, where exactly does that code go in the example to get it to work?

TorbenKoehn commented 8 years ago

Exactly where you current - $text = 'Test text' is

It's just important that you fill $__args['variableNameWithout$'] when you fill any $variableName.

You don't need to fill all of them, just those that you need in mixins.

I'm thinking about a better way constantly, but I just don't find any since you can basically use any PHP code in Tale Jade

jaydenseric commented 8 years ago

Crudely simplifying one of my page components, the first var_dump works but the second does not, it just renders NULL.

include ../main-aside/template.jade
include ../generic-content/template.jade

mixin post-page(props)
  - $__args['props'] = $props;
  +main-aside
    main
      - var_dump($props);
      +generic-content(['color' => $props['post']['color'])
        - var_dump($props);
jaydenseric commented 8 years ago

Hmm, pretty much all my component mixins accept a props parameter, my above attempt won't work because they will all be overwriting the global props; siblings will each overwrite the same props they are meant to share.

TorbenKoehn commented 8 years ago

This actually sucks. I'm currently experimenting with diffing get_defined_vars(), give me some minutes

TorbenKoehn commented 8 years ago

https://github.com/Talesoft/tale-jade/commit/eeaae938c60ae588c8db4626857f29d98ebc4fb2

Well I think that was too easy in my opinion, I don't believe that it works consistently haha (Not because I did something wrong, but because I don't believe that it actually was that easy)

It passes for sure.

Please do me a favor and checkout optimization/scoping and test your code with it (If you don't know how, just click on the branch and download the ZIP and unpack it into ./vendor/talesoft/tale-jade or do a proper git checkout optimization/scoping and git pull :))

jaydenseric commented 8 years ago

Thanks again for all you hard work, this one will be a very handy update 👍

Just got home from work (12:30 am here in Melbourne) and don't have the project/environment setup. I'll be sure to let you know how it goes, hopefully by Monday.

jaydenseric commented 8 years ago

Ok, so I have tested this out but there is still a problem.

mixin child(text)
  block

mixin parent(text)
  p=$text
  +child('Child text')
    p=$text

+parent('Parent text')

Should output, as Pug does:

<p>Parent text</p>
<p>Parent text</p>

But it instead outputs:


<p>
  Parent text</p>

<p>
  Child text</p>
jaydenseric commented 8 years ago

To give some context, we have a strict component architecture. Each UI component folder contains a JS file, CSS file and a jade template containing a mixin named after the component that accepts a props object as a parameter. Components can import other component mixins at the top with an include and use them internally. For example, a modal can import and use a button.

All of the properties needed to render a given page are set in a single props object in the route php file, which feeds this into the app component, which then goes on to recursively does it's thing.

This pattern has worked really well for us in Node.

jaydenseric commented 8 years ago

Playing around, this workaround works on the new branch:

mixin child(text)
  block

mixin parent(text)
  - $parentText = $text
  p=$parentText
  +child('Child text')
    p=$parentText

+parent('Parent text')
TorbenKoehn commented 8 years ago

Interesting. I'll work a bit more on this, but merging the scoping mechanism so far

TorbenKoehn commented 8 years ago

This is not possible easily in PHP.

What you can do is choose a different parameter name for the child-mixin. $text gets re-defined upon calling +child otherwise. In any other case you'd not be able the actual parameter of child anymore.

mixin child(someothernamethantext)
  block

mixin parent(text)
  p=$text
  +child('Child text')
    p=$text

+parent('Parent text')
jaydenseric commented 8 years ago

To give up on this and close the issue is to diverge significantly from the way pug renders. This project is labeled as "A complete and fully-functional implementation of the Jade template language for PHP". The chance that people would next mixins that have the same parameter name is really high, we encountered it immediately in our project but it took a long time to work out the issue.

TorbenKoehn commented 8 years ago

I'll keep it opened to point out that it's still an issue, but I currently see no good way around this. Maybe someone else does :)