Strict variables and custom filters in includes

vrugtehagel commented 8 months ago

Operating system

macOS Sonoma 14.2.1

Eleventy

2.0.1

Describe the bug

Eleventy fails to build if a custom tag is defined and used within a liquid include (if referenced with {% render '…' %}) while the strictVariables setting is switched on.

Reproduction steps

Here's an example setup. The folder structure is just that of a basic Eleventy project, with an include:

project/
├── _includes/
│   └── hello.liquid
├── index.liquid
├── package.json
└── .eleventy.js

The page to render, index.liquid, might look like

---
title: Hello!
---
{% render 'hello', name: 'Max' %}

The include, hello.liquid, is defined as

<div>
    Hello, {{ name | make_it_foo }}!
</div>

We are using a custom make_it_foo filter that turns anything into the string "foo". Of course, this is defined in our .eleventy.js config file:

module.exports = function(config) {
    config.setLiquidOptions({ strictVariables: true })

    config.addFilter('make_it_foo', function() {
        return 'foo'
    })
}

Note that we also have the strictVariables option switched on.

Expected behavior

I expect the build to succeed.

The build instead fails, with the following error:

[11ty] Problem writing Eleventy templates: (more in DEBUG output)
[11ty] 1. Having trouble rendering liquid template ./index.liquid (via TemplateContentRenderError)
[11ty] 2. undefined variable: page, file:/Users/koen/Desktop/11ty-filter-bug/_includes/hello.liquid, line:2, col:9 (via RenderError)
[11ty] 3. undefined variable: page (via InternalUndefinedVariableError)
[11ty] 
[11ty] Original error stack trace: InternalUndefinedVariableError: undefined variable: page
[11ty]     at Context._getFromScope (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:2487:23)
[11ty]     at _getFromScope.next (<anonymous>)
[11ty]     at toValueSync (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:504:32)
[11ty]     at toValueSync (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:510:25)
[11ty]     at Context.getSync (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:2469:16)
[11ty]     at Context.get (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:2466:21)
[11ty]     at Object.<anonymous> (/Users/koen/Desktop/11ty-filter-bug/node_modules/@11ty/eleventy/src/Engines/Liquid.js:59:34)
[11ty]     at Filter.render (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:2144:35)
[11ty]     at render.next (<anonymous>)
[11ty]     at toPromise (/Users/koen/Desktop/11ty-filter-bug/node_modules/liquidjs/dist/liquid.node.cjs.js:479:32)
[11ty] Wrote 0 files in 0.02 seconds (v2.0.1)

In human words, it is failing because Eleventy is trying to make the page variable available to the function defining the custom filter (i.e. in this.page), but in the included template, the page variable does not exist. While the page variable is not used anywhere explicitly, the strictVariables mode flags this as an issue due to Eleventy itself attempting to access page.

vrugtehagel commented 8 months ago

For what it's worth, simply changing the page and eleventy properties into getters does work.

Specifically, changing these lines into

const exposedProperties = ["page", "eleventy"];
for (const property of exposedProperties) {
  Object.defineProperty(this, property, {
    configurable: true,
    enumerable: true,
    get: () => this.context.get([property]),
    // perhaps a setter, though there's probably no point
  })
}

Presumably it needs to be updated for the other templating engines too (not just liquid), though, so this by itself is probably not satisfactory.

Happy to open a PR for this, if you'd like!

AleksandrHovhannisyan commented 7 months ago

Howdy, I believe this is a duplicate of this issue: https://github.com/11ty/eleventy/issues/2453. That issue describes a bit more in detail why this bug occurs.

vrugtehagel commented 7 months ago

It's not - that issue describes using variables inside {% render %}-style partials, and is not really related to filters. This bug specifically is about using filters that are not using the globals variables but still fail inside {% render %}-style partials.

AleksandrHovhannisyan commented 7 months ago

I think the underlying reason is still the same: 11ty doesn't register page and other globals in Liquid (which is #2453). If it did I don't think you'd see this issue.

vrugtehagel commented 7 months ago

That's true, though that's out of scope for this issue, which has different semantics and accompanied by a simpler fix. Exposing the eleventy-provided globals to rendered partials may not even be bug necessarily since the whole point of {% render %}-style partials is encapsulation, whereas filters should not break this way regardless.

If #2453 is addressed and the relevant globals are exposed to partials, then this issue disappears, but I do think they are separate issues regardless.

11ty / eleventy