mozilla / nunjucks

A powerful templating engine with inheritance, asynchronous control, and more (jinja2 inspired)
https://mozilla.github.io/nunjucks/
BSD 2-Clause "Simplified" License
8.58k stars 640 forks source link

Add reference to original source input in env.on('load') emit #1303

Open rdmurphy opened 4 years ago

rdmurphy commented 4 years ago

Hello! 👋

I've been trying to bend the env.on('load') functionality into a situation where I can determine exactly what files were imported per initial source file, and I think short of constantly setting up and tearing down a new listener for each input it's not currently possible. (And that'd fall apart in any async situations and require each run to sequentially run through the inputs, because otherwise the listeners may overlap.)

I wanted to float the idea of having what is emitted in env.on('load') contain more context about the environment a template was loaded in. I think the only catch is it'd require some parameters to be passed into getSource that'd have no other purpose other than to be logged by loader.on('load'). I was imagining something like initialSrc or a better named key being added to the second object here. This could even go as far as adding an additional value called parentSrc that refers to the template that directly import it, which would make it possible to actually build the chain of request.

The reason I'm after this is because I want to have a watcher that is smart about only rebuilding pages that actually use a given template, instead of re-rendering every single page just because a given template may have imported that template downstream.

rdmurphy commented 4 years ago

Another much bigger suggestion/change would be to have render() return an object instead of the rendered string, which would enable a much more rich set of context to travel with each output. This is how dart-sass and node-sass handle it, for example.

ogonkov commented 4 years ago

I'm using nunjucks parser to achieve same dependency tree for my webpack loader.

I think it's would be pretty easy to add parent template to second arg of load event.

Changing render() signature could happen with some option, it's major breaking change, and useless for most users, i guess.

rdmurphy commented 4 years ago

Thank you for the response!

I think it's really cool and clever how you're using the nunjucks parser itself to determine the dependencies, but I feel like that's a lot of work to implement and maintain when nunjucks itself could potentially surface the same information.

(I'm also aware of emitty which does something similar to use the parser to determine what files are used.)

I do want to slightly push back at the idea this would be useless for most users — I expect most are using a wrapper or tool (like your nunjucks-loader) or the CLI to accomplish what they need. So this could be a great addition for tool builders (like you and I) and in the majority of cases never be something users have to worry about because most are not calling render themselves already.

If we truly didn't wanna risk a breaking change (which is totally fair!) I could additionally see this as an entirely different render call (something like renderWithStats or similar) or as a flag that gets passed to render to optionally return the extra data, but otherwise it functions as normal.

ogonkov commented 4 years ago

I think method like renderWithStats is totally possible. Can you guess what is returned data should look like?