Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define global objects which can be accessed by render tag #185

Closed
paulrobertlloyd opened this issue Feb 5, 2020 · 9 comments
Closed

Define global objects which can be accessed by render tag #185

paulrobertlloyd opened this issue Feb 5, 2020 · 9 comments

Comments

@paulrobertlloyd
Copy link
Contributor

paulrobertlloyd commented Feb 5, 2020

Having experienced a few cases where variables have unintentionally leaked into included template partials, it was interested to note that Liquid (and LiquidJS) now support the stricter render tag, and that include has been deprecated.

I tried to update my 11ty-generated site to use render, but in doing so, soon came across a limitation; global data objects are not available to rendered partials.

For example, I have the following partial:

<header class="banner">
  <div class="banner__container">
    <div class="banner__title">
      <a rel="home" href="/">{{ app.title }}</a>
    </div>
    <nav class="banner__navigation" aria-label="site">
      {%- render 'navigation' with navigation.primary -%}
      {%- render 'navigation' with navigation.secondary -%}
    </nav>
  </div>
</header>

The data for both app and navigation comes from global data files. Given the encapsulated nature of render, I have to declare these each time I wish to render this partial on a page, which is not always easy or possible within the confines of a Liquid template.

Looking at the relevant Shopify theme docs, I note that:

Global objects don't need to be passed down. They are accessible from all files.

Shopify define their own global objects, but there’s no way for an author using LiquidJS to do the same, as far as I can see.

I imagine being able to define an array of globals when configuring LiquidJS (shown within the context of an 11ty configuration), like so:

const {Liquid} = require('liquidjs');
const app = require('./src/_data/app.json');
const navigation = require('./src/_data/navigation.json');

module.exports = function(eleventyConfig) {
  let options = {
    extname: '.liquid',
    strict_filters: true,
    root: [
      './src/_includes',
      './src/_layouts'
    ],
    globals: [
      app,
      navigation
    ]
  };

  eleventyConfig.setLibrary('liquid', new Liquid(options));
};

Alternatively, perhaps an object would allow authors to define a name for each global:

globals: {
  foo: app,
  bar: navigation
}

Not sure which is the best approach; I have no preference.

Is this a legitimate feature request, or have I missed something?

@paulrobertlloyd paulrobertlloyd changed the title Define global data objects which can be accessed by render tag Define global objects which can be accessed by render tag Feb 5, 2020
@harttle
Copy link
Owner

harttle commented Feb 6, 2020

At first glance it seems something contradictory: we need to make the variables passed down explicit, and we need globals at the same time. But I'll try to be compatible with the shopify liquid and dig into that behaviour to make sure it's intended. And I prefer to the second way (in which you can define a object literal to specify the globals).

harttle pushed a commit that referenced this issue Feb 7, 2020
# [9.7.0](v9.6.2...v9.7.0) (2020-02-07)

### Bug Fixes

* expression and string literal parser, [#186](#186) ([fc0cf6f](fc0cf6f))

### Features

* globals shared between tags, see [#185](#185) ([870e7ec](870e7ec))
@harttle
Copy link
Owner

harttle commented Feb 7, 2020

Try [email protected], it should be supported by passing globals option.

@paulrobertlloyd
Copy link
Contributor Author

Thanks @harttle; started working with this, and all seems to work as expected. Will file any bugs/issues if I come across them. Thanks so much for adding this feature so quickly!

@zeroedin
Copy link

@harttle is there a way to get globals option to work with Express Liquid integration? https://liquidjs.com/tutorials/use-in-expressjs.html

I feel like i'm going down a rabbit hole here, I understand the express res.render method does not take the same params as the liquid.renderFile method, so trying to work my way around that. Any ideas? Maybe I am I overlooking something but I think i'm at the point where using res.render is out of the question for Liquid global variables.

@zeroedin
Copy link

I think I might have found a solution:

  let engine = new Liquid({
    root: path.join(__dirname, 'views'),
    extname: '.liquid'  // used for layouts/includes, defaults ""
  })
  app.engine('liquid', engine.express({}))

  app.set('views', path.join(__dirname, 'views'))
  app.set('view engine', 'liquid')
  
  let html = engine.renderFileSync(path.join(__dirname, 'views','index'), {
    data: 'local'
  }, {
    globals: { data: 'global' }
  })

  res.send(html)

To use in an express router I had to set app.local.engine = engine in a app.js middleware and use app.locals.engine.renderFileSync

@harttle
Copy link
Owner

harttle commented Jan 12, 2021

The globals can be set when creating anengine object.

let engine = new Liquid({
    root: path.join(__dirname, 'views'),
    extname: '.liquid'  // used for layouts/includes, defaults "",
    globals: {data: 'global'}
  })

@jsuiker
Copy link

jsuiker commented Dec 1, 2023

the settings object indeed does not work when using express res.render method

@jsuiker
Copy link

jsuiker commented Dec 1, 2023

@harttle please see the following repo for a minimal example of the settings object NOT working when using express res.render()

https://github.com/jsuiker/liquidjs-express-globals

@harttle
Copy link
Owner

harttle commented Dec 2, 2023

@jsuiker I find expressjs passes a settings variable when calling render engines, and local scope has higher priority than globals. Sadly I don't find any docs from its website. But if you delete that from locals, your demo will work fine:

  var app = express()
  // after `express` is created, delete its `.locals.settings`:
  delete app.locals.settings;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants