pugjs / pug

Pug – robust, elegant, feature rich template engine for Node.js
https://pugjs.org
21.68k stars 1.95k forks source link

Inclusion should create a new scope #2027

Open smaili opened 9 years ago

smaili commented 9 years ago
//- includer.jade
- for ( var i = 0; i < 2; i++ ) {
-   console.log( 'includer' + i );
include /includee
- }
//- includee.jade
-
  for ( var i = 0; i < 5; i++ ) {
    console.log( 'includee' + i );
  }

Output:

includer0
includee0
includee1
includee2
includee3
includee4

As you can see, includer never gets past the first iteration due to the includee overwriting the value of i, which should not happen since includee's var i = 0 should create a new scope.

ForbesLindesay commented 9 years ago

Include currently intentionally doesn't create a new scope, to allow you to "include" files containing variable declarations, function declarations, and mixins. Unfortunately, this leads to the problem you are describing. I think the best non-breaking ways of fixing this will be via the proposed import syntax, which is more akin to ES6's import syntax, and via ES6's let keyword, which we should support in the future, and which could be made to follow the more restrictive scoping rules.

smaili commented 9 years ago

@ForbesLindesay, what's wrong with just checking whether there is a var before the variable name? Or is that easier said than done?

ForbesLindesay commented 9 years ago

True, if we did scope var locally to the include, you could still assign without the bar to expose something to the rest of the template. It would be a huge breaking change at this point though.