Add async webpack.resolve support #27

ThaNarie · 2017-03-12T14:00:36Z

Switch the compiler to async mode, to allow webpack to resolve the
included resources. The compilation is done in multiple passes until
all paths are resolved (most of the time just 1 extra pass).

Change the compiler export to a factory function so data can be passed
from the loader to the compiler. Because of this structure the mapcache
is not needed anymore.

Create a util function to correctly generate template IDs that are the
same between the loader and the compiler (previously they could differ).
They always use the resolved webpack path as source for the ID.

Skip dependencies in the token parsing that are not just a string. They
will be handled at runtime, and are the responsibility from the template
author.

Also export the template tokens on the template function, so they can be
used to register the template under a different name at runtime. This is
useful when dealing with dynamic templates where you want to use
require.context to bundle and register all your templates in your
application bootstrap.

Updated the test helper to make empty context available.

Added a lot of comments in the code.

re #26

Switch the compiler to async mode, to allow webpack to resolve the included resources. The compilation is done in multiple passes until all paths are resolved (most of the time just 1 extra pass). Change the compiler export to a factory function so data can be passed from the loader to the compiler. Because of this structure the mapcache is not needed anymore. Create a util function to correctly generate template IDs that are the same between the loader and the compiler (previously they could differ). They always use the resolved webpack path as source for the ID. Skip dependencies in the token parsing that are not just a string. They will be handled at runtime, and are the responsibility from the template author. Also export the template tokens on the template function, so they can be used to register the template under a different name at runtime. This is useful when dealing with dynamic templates where you want to use `require.context` to bundle and register all your templates in your application bootstrap. Updated the test helper to make empty context available. Added a lot of comments in the code. re zimmo-be#26

webberig · 2017-03-12T16:04:12Z

Hi Arjan, Thanks for your contribution! I have a few remarks though: - can you please make sure the existing tests pass? - could you add some test cases covering the scenarios why you're making these additions (ie. Template A referring to template B using a variable of sorts) - can you also fix the indenting in compiler.js? The changes look a bit messy now :-) Op 12 mrt. 2017 3:00 p.m. schreef "Arjan van Wijk" <notifications@github.com

…

: Switch the compiler to async mode, to allow webpack to resolve the included resources. The compilation is done in multiple passes until all paths are resolved (most of the time just 1 extra pass). Change the compiler export to a factory function so data can be passed from the loader to the compiler. Because of this structure the mapcache is not needed anymore. Create a util function to correctly generate template IDs that are the same between the loader and the compiler (previously they could differ). They always use the resolved webpack path as source for the ID. Skip dependencies in the token parsing that are not just a string. They will be handled at runtime, and are the responsibility from the template author. Also export the template tokens on the template function, so they can be used to register the template under a different name at runtime. This is useful when dealing with dynamic templates where you want to use require.context to bundle and register all your templates in your application bootstrap. Updated the test helper to make empty context available. Added a lot of comments in the code. re #26 <#26> ------------------------------ You can view, comment on, or merge this pull request online at: #27 Commit Summary - Add async webpack.resolve support File Changes - *M* README.md <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-0> (59) - *M* lib/compiler.js <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-1> (155) - *M* lib/loader.js <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-2> (97) - *D* lib/mapcache.js <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-3> (2) - *A* lib/utils.js <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-4> (25) - *M* package.json <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-5> (3) - *M* test/fakeModuleSystem.js <https://github.com/zimmo-be/twig-loader/pull/27/files#diff-6> (3) Patch Links: - https://github.com/zimmo-be/twig-loader/pull/27.patch - https://github.com/zimmo-be/twig-loader/pull/27.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#27>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADSVlK_zxTFfDRM6jBb0IDLuwkQO6-5rks5rk_qEgaJpZM4MajCm> .

ThaNarie · 2017-03-12T16:38:12Z

failed tests: Sure! Forgot to look at the Travis result (they passed locally on node 6 :P), I see we still need to support 0.10 :) (without starting a discussion, isn't this something we should drop, as 0.12 was already end-of-life at 2016-12-31)
extra tests I will give it a try, I hope the current 'mock' setup can handle the new webpack resolve features.
indenting Ah sure, I'll also add a .editorconfig so the next person won't make the same mistake by accident. Also, the original indenting for the /lib files have 4 spaces, but the tests have 2 spaces, I've now chosen 4...

Mark a failed resolve as false, so the compiler can ignore that resource and leave it in the template as is, so it can be picked up by Twig at runtime.

When not including `.twig` as extension, it will be added automatically. This is to create a scenario where a file can be resolve by webpack, and two different paths lead to the same resource. Also add a check on disk to allow cases where a template include value only exists at runtime as registered template.

When adding the behavior where a template must exists on disk to have it processed by the loader, the files do need to exist in disk. Also changed the include html to work with a resolve case where the '.twig' extension is added by the webpack resolve function.

These new tests will all fail when run on the old code.

ThaNarie · 2017-03-12T18:38:15Z

@webberig All your remarks have been addressed. The test setup is not ideal, but it simulates some of the webpack resolve features that show how the old/current version is breaking, and where this PR passes.

I have tested this version in my own project, but that is very limited (basically only a for loop with dynamic includes and some other includes, no blocks or embeds or template extends). If you have some projects where you use this library, I think it's wise to install this PR version (my branch) to see if those other cases behave the same as well.

The package.json version is left the same, not sure how you want to handle the release :)

JohnAlbin · 2019-05-27T02:56:00Z

README.md

@@ -26,6 +28,26 @@ module.exports = {
 };
 ```

+### Webpack 2


Since Webpack 2 and later is more likely, how about this goes before the "Webpack 1" docs? Also, you'll need to say "and later" since many are using version 3 or 4.

And, yeah, I know you wrote this PR 2 years ago. :-p

JohnAlbin · 2019-05-27T03:05:35Z

lib/compiler.js

@@ -1,62 +1,102 @@
 var path = require("path");


The PR lists several files, including this one, as having “conflicts that must be resolved”.

Hi @JohnAlbin,

I'm happy to make the required changes, but it would be good to know if this PR is going to be used at all in any way, since the original author seems afk after I made the requested changes :)

Let me know! Cheers

# Conflicts: # README.md # lib/compiler.js # lib/loader.js # package.json

webberig

Sorry for the late reply. I've taken another look at the changes and will release this under version 0.5.0

ThaNarie added 6 commits March 12, 2017 17:42

Add .editorconfig and revert to original formatting style

182ce70

Correctly handle case where webpack cannot resolve a file

e5b16a4

Mark a failed resolve as false, so the compiler can ignore that resource and leave it in the template as is, so it can be picked up by Twig at runtime.

Add tests to verify webpack.resovle behavior

6c18c06

These new tests will all fail when run on the old code.

Change includes to something it that is supported in node 0.10

e737850

JohnAlbin suggested changes May 27, 2019

View reviewed changes

ThaNarie added 3 commits June 9, 2019 20:01

Merge remote-tracking branch 'upstream/master' into feature/resolve

a3401a9

# Conflicts: # README.md # lib/compiler.js # lib/loader.js # package.json

Add missing tokens after merge

024853c

Update readme for webpack 2+

fa18668

webberig approved these changes Jun 10, 2019

View reviewed changes

webberig merged commit 076d990 into zimmo-be:master Jun 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add async webpack.resolve support #27

Add async webpack.resolve support #27

ThaNarie commented Mar 12, 2017

webberig commented Mar 12, 2017 via email

ThaNarie commented Mar 12, 2017 •

edited

Loading

ThaNarie commented Mar 12, 2017

JohnAlbin May 27, 2019 •

edited

Loading

JohnAlbin May 27, 2019

ThaNarie Jun 9, 2019

webberig left a comment

Add async webpack.resolve support #27

Add async webpack.resolve support #27

Conversation

ThaNarie commented Mar 12, 2017

webberig commented Mar 12, 2017 via email

ThaNarie commented Mar 12, 2017 • edited Loading

ThaNarie commented Mar 12, 2017

JohnAlbin May 27, 2019 • edited Loading

Choose a reason for hiding this comment

JohnAlbin May 27, 2019

Choose a reason for hiding this comment

ThaNarie Jun 9, 2019

Choose a reason for hiding this comment

webberig left a comment

Choose a reason for hiding this comment

ThaNarie commented Mar 12, 2017 •

edited

Loading

JohnAlbin May 27, 2019 •

edited

Loading