pugjs / pug-loader

Pug loader module for Webpack
MIT License
425 stars 119 forks source link

[BUG] Template scope variables are not passed to the include template #54

Open antpaw opened 7 years ago

antpaw commented 7 years ago

In this example you would expect that myScopedVar is available in low_level_template.pug.

for myScopedVar in [1, 2, 3] 
  include low_level_template

But it's not. That's a huge deal for me because that's how it works in your node module used for expressjs. That's also how most people assume it should work 1 2. Also that's how jadeify does it.

Here is a testcase example (please follow the steps from readme.md) and watch the browser console output. The problem that the isomorphic_template gets compiled with this line:

for (var pug_index0 = 0, pug_length0 = pug_obj0.length; pug_index0 < pug_length0; pug_index0++) {
   var myScopedVar = pug_obj0[pug_index0];

   pug_html = pug_html + (null == (pug_interp = __webpack_require__(9).call(this, locals)) ? "" : pug_interp);
}

it's clear to see that myScopedVar is set but not provided to the module 9 (which is the low_level_template.pug file). Jadeify would "inline" this template so the var scope is still present.

antpaw commented 7 years ago

I've found this answer https://github.com/pugjs/pug-loader/issues/47 I strongly disagree with this idea.

This is a important template feature and using locals is not documented in any way in pug/jade documentation. Setting something on a global object locals will always lead to side effects that might cause unexpected behaviour. It might be not a big deal for client side, but in a isomorphic app where partials have to work on node as well you might be modifying unintentionally object references for different requests.

The best fix for this would be if pug had two different syntaxes for include that let programmers decide to inline or to require the template. Or a syntax that provides a way to define variables that should be passed down to the include along with already set variables.

Chances for this to happen are almost zero i would say, so I suggest that the library should inline every "include".

TimothyGu commented 7 years ago

I guess @sokra (who initially implemented template inlining) didn't realize this incompatibility. I agree with your evaluation, and I'm okay with the idea of adding a require plugin in pug-loader. But until such a keyword is implemented, I'm inclined to keep the status quo as it is, so that existing Jade (and jade-loader) users can more easily upgrade to Pug.

ryoikarashi commented 7 years ago

any update?

antpaw commented 7 years ago

@RyoIkarashi you can use my fork for now https://github.com/antpaw/pug-loader

forbaz commented 7 years ago

Hello everyone. I don't know why this happen, but if in included file we call any mixin variables that we passed returned their value. For example:

//- empty.pug
mixin empty()
  if false
    p nothing here
//- template1.pug
include empty.pug
for myScopedVar in [1, 2, 3]
  include low_level_template
//- low_level_template.pug
+empty()
p= myScopedVar

And this work. But if we just delete line with +empty() mixin call, myScopedVar return undefined. Maybe this can help someone.

damassi commented 6 years ago

@TimothyGu - Any tips as to how to go about implementing this would be appreciated. This issue just put a hard stop on our large webpack refactor

antpaw commented 6 years ago

@damassi this is the fork i have used to transfer my ezel based setup to webpack. The only downside I can see is that the bundle size will get bigger (extremely bigger, in extrem cases), but it uses the same convention as the browserify compiler ect, so coming from other compilers should not cause noticeable size difference. But it could be done more efficient if you would somehow "collect" all the variable form the closer scope and pass them to the included template function instead of inlining it. I think this will require a major change in the internal workings of pug, and am not even sure if the "collecting" part is possible.

damassi commented 6 years ago

Thanks @antpaw - will look into it!

damassi commented 6 years ago

@antpaw - Can confirm your fork fixed the issue. Thanks again 👍

DevanB commented 6 years ago

Any movement on getting this fixed?

AmirTugi commented 5 years ago

For those of you who seek a hack for the for loop - the solution in #47 works if you define the var in the locals:

- locals namesArray = ['Amir', 'Tugi']
- each element in locals.namesArray
    - locals.element = element
    include path/to/file // <-- The file will now be able to access the variable via #{element}

With that said, any progress on the real official solution yet?

CharlesGouldmann commented 4 years ago

Any progress on this?

For now I will use @antpaw 's fork which seems to work great.

titantwentyone commented 4 years ago

It would be great if the docs could be updated to relfect this. Took me a while to figure out what was happening here. @AmirTugi's fix worked perfectly for me.

webdiscus commented 2 years ago

Following simple example work with the pug-loader:

- var items = ['Hello', 'pug!']
each str in items
  include ./include-var-str

file include-var-str.pug:

i= str

The local variable str defined in each in cycle is available in included template.

Output:

<i>Hello</i><i>pug!</i>