Dynamic include and pre-register templates

[ThaNarie]

TLDR;

• When using dynamic includes the loader breaks --- could ignore it and leave the original value.
• The loader always tries to replace and resolve the passed value for include (even when it cannot be resolved by webpack / doesn't exist on disk), so when you want to use pre-registered templates (at runtime), Twig cannot find them and require will give an error --- could check webpack resolve and only replace/require when can be resolved

Dynamic includes

I've been trying to find a way to make dynamic includes work.

{% for block in blocks %}
  {% include './blocks/' ~ block.name ~ '.twig' %}
  {% include block.name %}
{% endfor %}

First of all, the above fails horribly since an undefined value is passed here (binary and dynamic tokens), and will later error if that value is passed in JSON.parse.

The current code only can deal with a single string value as include path.

If you look at this from the webpack point of view it sort of makes sense. It's not really possible to require and bundle a dynamic path, since that path is only know during runtime. For this you would need to create a require context (that bundles all available matches to the pattern), but that only works when you manually pass that dynamic string in a webpack.context line, and that is also not possible when working with dynamic strings for the tokens. Also it might complicate things even more.

Pre-register templates

So after that I started looking into how templates are resolved, which is by creating a hash of the filepath, so the registered template can be found later when it is included.
This means that if I know the 'dynamic' values that will come out of the expression beforehand, I could register twig templates manually (by creating a webpack context myself), so they are all available at runtime.

E.g., if I would execute this code before I would render my main template:

var template = twig({
	id: './blocks/block1/twig',
	data: 'foo bar',
	allowInlineIncludes: true,
	rethrow: true
});

And the block.name in the example above would be "block1", the include {% include './blocks/' ~ block.name ~ '.twig' %} would resolve at runtime, and the template would be available.

So I think that if it would just ignore dynamic values and let them be handled at runtime, the above scenario could work.

Use webpack resolve instead of path.resolve

Another thing, when we are passing strings, I think we could improve that scenario case as well.
Currently it's making use of path.resolve, but instead if we would pass the loaderApi from the loader.js to the compiler instance (by using a factory function), it could use the resolve method on that:

loaderApi.resolve(loaderApi.context, token.value, function (err, result) {
  // push to includes array & update token value
}

Now only paths that can be resolved will be added as requires, so unknown paths will not throw runtime require errors, and left to be dealt by Twig itself . This allows us to register any named template and use that string in an include.

Async loader

Implementing this change also requires the compiler to be async (due to the resolve being async, the sync version will throw webpack errors when the webpack filesystem is not in sync), but that's not really possible.
I looked at how the handlebars-loader handles this, and they seem to do a multipass compilation until all the resolves are completed.

We might be able to do the same thing if we keep a map of all the resolved paths based on their token.value. On the first pass we could start the resolve and keep the original token value, and on the second pass it would ignore the hash-like values it already replaced on the first pass, and update the token values with the resolved values from the map.

Final words

Please let me know how you feel about the above, and if I'm on the right track here. Would it be useful to start a PR with a (reference) implementation, or a waste of time if will not be used?

In #20 it was mentioned that it's better to use the webpack resolve for things like namespace paths, but I think the above is something different (since it would need totally skip the require in some cases and leave the original value).

[ThaNarie]

Also, related to #20, It seems there might be a mismatch between the loader and compiler when they generate the id.

The compiler is doing token.value = hashGenerator(path.resolve(path.dirname(resourcePath), token.value)) where token.value can be a 'shorthand' that will be resolved differently by webpack (e.g. by omitting the extension, or by using the DirectoryNamedWebpackPlugin).

However, the loader is doing hashGenerator(require.resolve(this.resource)), where this.resource is a already resolved string (so the full path to the resources).

Since both values generate a different hash, this will fail at runtime.

If the above is indeed an issue, it would be another reason to use loaderApi.resolve during the compilation phase.

[ThaNarie]

Regarding the Pre-register templates section above, I made an assumption that didn't check out. Currently it's not really possible to load a template via webpack, and register that template under a different name, since you cannot access the data property from a loaded template (function).

There might be a workaround for it, but that would require a change in the current implementation.

1. also export the tokens in the twig module (e.g. as property on the function), so that can be used to re-register the template.

2. or, to make use of twig({ref: 'foobar'}) where 'foobar' is the id of the previously registered template, that will return the template object including the used tokens, which can be used to re-register the template.
   However, since the the hashing algorithm is something internal, and uses the local resolved filepath that is not really available at runtime, this is not really an option at the moment.
   If we would use a different source for the hash (or also don't generate the hash at all), it would be easier to retrieve the template using the ref method.

For reference, the handlebars-loader returns an anonymous template-function and uses inline requires to load/execute partials, so we cannot use the same thing as has been done there.

[MXTcomunica]

Any news on this?

In my application managed by webpack and twig-loader I'm able to include twig templates one inside the other using relative paths, e.g, consider this twig file:

<div>
  <h1>Lorem ipsum</h1>
  {% include '../../01-atoms/button/button.html.twig' %}
</div>

The above works well, but when I try to create alias in webpack.config.js in this way:

resolve: {
    alias: {
      atoms: path.resolve(__dirname, 'src/components/01-atoms'),
      organisms: path.resolve(__dirname, 'src/components/03-organisms')
    }
  }

To be used for example in this way:

<div>
  <h1>Lorem ipsum</h1>
  {% include '@atoms/button/button.html.twig' %}
</div>

Twig.js gives an error and cannot resolve templates inclusion.

How to resolve this?

Thank you very much

[webberig]

Hey all,

Sorry I haven't had the time to respond to this sooner. There are different topics here at discussion.

Dynamic includes

Since the twig-loader is a webpack plugin, I strongly believe it needs to stay as close as possible to how webpack works, even though TwigJS may support other solutions.

As @ThaNarie pointed out, webpack doesn't do dynamic includes, unless you use webpack.context. So, unless we find a way to make the loader generate a working webpack.context definition, we must accept the fact that this isn't possible in a webpack environment.

Pre-register templates

I fail to see a use case how you would end up in this scenario, but I'm open to suggestions on how we can improve the way twig-loader adds a proper reference to each template. TwigJS has its own reference and cache system that isn't the best solution when using it in combination with webpack, so I needed to be creative to make references between templates work.

Use webpack.resolve

I don't fully understand what you mean by passing a string and "that scenario". I don't see a practical scenario why or when webpack.resolve would be better, but I guess in theory it could make more sense.

Async loader

This could be explored to perhaps increase build performance, but I don't think that supporting dynamic includes is the proper argument to implement this.

compiler/loader mismatch

Feel free to improve if you happen to find bugs. Try to prove the bugs using unit tests if possible

Alias support

This might be a good argument to try using webpack.resolve instead of path.resolve. Feel free to create a PR for this

In conclusion, if you like to contribute, please keep the PR's simple and as small as possible with a clear use scenario or proof why this is an improvement. The reason why your PR has not been merged yet, is because it results in a complete overhaul of code that I don't fully understand why.

[webberig]

Closing this issue as it's too monolithic... Feel free to open new issues for very specific problems :-)