Support customers level template bundles

[tshamz]

What are you trying to accomplish with this PR?

After allowing alternate template bundles in beta.8, the liquid logic included in script-tags.liquid snippet to check which bundles to load and which bundles to prefetch, was updated to use template from template.name. This creates a problem for any template that's included in the customers/ directory because the value that's used to create the template name in the liquid logic is taken from the chunk inside the webpack plugin. This is a problem because bundles/entry points inside the customers/ directory are flattened down to the same level as the other files, which produces files like this:

template.login.js
template.account.js
template.order.js

Those file names are used to create the blocks inside of script-tags.liquid. This used to be ok before because if you were on a customers/ template e.g. login.liquid, {{ template.name }} would produce 'login', but now since switching to {{ template }} the output has changed to 'customers/login'. The resulting output looks like this:

{%- if template == 'login' -%}
    <script type="text/javascript" src="https://10.10.10.239:3001/template.login.js" defer="defer"></script>
{%- else -%}
    <link rel="prefetch" href="https://10.10.10.239:3001/template.login.js" as="script">
{%- endif -%}

The problem here is that template == 'login' will never evaluate to true, so this bundle will never get loaded. This pull requests alters the script-tags.html template to check if the chunk includes customers/ in its path, and if it does, prepends customers/ to the template check inside of the liquid logic.

Additionally, after poking around a bit, I noticed that bundles for alternate templates in the customers/ directory weren't being loaded in due to a conditional inside of get-template-entrypoints.js. Templates not inside customers/ are getting run through a new isValidTemplate function, but the customers/ templates are not. As a result, any alternate customers/ template is not being seen as valid, and thus not being included. This pull request updates get-template-entrypoints.js to replace the old valid template check with isValidTemplate that allows alternate templates to pass through.

Please provide a link to the associated GitHub issue.
#806

Checklist

For contributors:

• I have updated the docs to reflect these changes, if applicable.

For maintainers:

• I have 🎩'd these changes.

[t-kelly]

Hey @tshamz! Great PR! Was able to give it a test run, everything you proposed to fix is fixed and works great. This PR is good to merge.

However, I found a few more holes our logic:

1. If a customer template bundle shares code with another template bundle, the chunk created still results in a liquid condition that doesn't include customers/ prefix, e.g:

{%- if template == 'index' or template == 'login' or template == 'product' -%}
<script type="text/javascript" src="{{ '[email protected]@template.product.js' | asset_url }}" defer="defer"></script>
{%- else -%}
<link rel="prefetch" href="{{ '[email protected]@template.product.js' | asset_url }}" as="script">
{%- endif -%}

2. The problems you fixed still exists in style-tags.liquid, the snippet for including our stylesheet bundles.
3. Problem 1 written above still exists in style-tags.liquid

If you wish to expand this PR to fix those problems, that would be 💯. If you'd rather not, we can merge this and I'll create followup issues to tackle the additional changes.

There have been a number of bugs associated to these snippets. They are super powerful and would really benefit from some tests to guarantee stability. Again, not needed for this PR but I can create a separate issue.

[tshamz]

I figured number 1 was gonna be a problem at the time. I think underscore templates are a nightmare to read and I had already been working for a while when I ran into this so I didn't really have the mental capacity to try and decipher what was going.

I added a fix for this and copied the same logic over to the style-tags.html file as well. I did not make it around to writing some tests but I'll noodle on it and if i have some spare time put something together.

[t-kelly]

@tshamz awesome! I'll give this a test run and we can merge it as is. I'll create a followup issue about the shared chunks. Oh damn, read that wrong -- you did get shared chunks! 💯

[t-kelly]

Gave it a test. Looks solid. Awesome contribution! Thanks @tshamz

No worries about the tests. Something we can chip away at over time.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.