Allow for subdirectories in snippets/ and sections/

[tshamz]

What are you trying to accomplish with this PR?

Allow for subdirectories to be used in the snippets/ and sections/ directories by updating the core.js webpack config in slate-tools to use a separate a file-loader rule when dealing with these two directories.

The resolution order for how namespace collisions are handled is done alphabetically by directory name (with the last directory winning), and from topmost directory to bottom (with the topmost directory winning). For example:

• sections/test.liquid beats sections/aaa/test.liquid
• sections/zzz/test.liquid beats sections/aaa/test.liquid
• sections/zzz/zzz/test.liquid beats sections/zzz/aaa/test.liquid
• sections/zzz/test.liquid beats sections/zzz/zzz/test.liquid

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

Checklist

For contributors:

• I have updated the docs to reflect these changes, if applicable. (I'm not entirely sure where this should go but I think the explanation of the resolution order should suffice)

For maintainers:

• I have 🎩'd these changes.

[dan-gamble]

Should this be able to work like {% include 'collections/hero' %} or does the include still need to be written as just {% include 'hero' %}?

[tshamz]

my preference would be for {% include 'hero' %} so that people could write snippets in a style where they initially inherit a core set of snippets and then if they want to use their own, different version of the snippet, instead of creating a snippet with a different name and then updating the reference/include everywhere in their liquid, they just create another one with the same name that overwrites the core one and is automatically used instead, similar to how something like oh-my-zsh handles custom plugins/themes.

[t-kelly]

My hope is that we can eventually reference snippets and sections with relative paths like:

{% include '../snippets/collections/hero.liquid' %}

And then on build replace the path with resolved path:

{% include 'hero' %}

This would allow us to resolve naming conflicts internally and never let the developer worry about them. For example, if we had these two includes:

{% include '../snippets/hero.liquid' %}
{% include '../snippets/collections/hero.liquid' %}

they would be resolved to the following on build:

{% include 'hero' %}
{% include 'hero2' %}

Thoughts?

This PR is moving in that direction, so I'm not opposed to the solution proposed here for now.

[tshamz]

tbh, i'm not really that attached to this pull request since everything that it does and that I want it to do can be accomplished from within slate.config.js. As I briefly alluded to earlier, what I'm looking to accomplish is set up a base theme that comes with a core set of snippets, let's call them "snippet components, e.g, button.liquid, link.liquid, input.liquid, hero.liquid, list.liquid, option-group.liquid, etc., which are located in snippets/core/, and represent the different components used when building a store.

Then, when a developer starts building a new theme, they get all the core snippets and can use them to build out their store, but if they need a component that differs wildly from the version that core snippet component offer, they would made a snippet with the exact same name in a different directory (e.g. snippets/core/button.liquid and snippets/custom/button.liquid).

Like I mentioned, I can get all of this already with a few modifications to my slate.config.js, so it's not a huge bummer to me if this doesn't get merged in. I think the conversation about path resolution and name collisions is interesting and valuable, but not really something this PR set out to accomplish.

[t-kelly]

Going to let this one brew for a little since it's not critical and as you mentioned, it can be achieved via the Slate config.

but if they need a component that differs wildly from the version that core snippet component offer, they would made a snippet with the exact same name in a different directory (e.g. snippets/core/button.liquid and snippets/custom/button.liquid).

What's the advantage of this over just modifying the original? What you're proposing sounds like Wordpress child themes, which existed to facilitate versioning the core/parent theme. Shopify themes don't really have anything similar to this, nor does Slate?

I think the conversation about path resolution and name collisions is interesting and valuable, but not really something this PR set out to accomplish.

True. Thanks for clarifying further your goal with this PR.

[dan-gamble]

@t-kelly i'm with you in that i definitely feel you should be using some sort of patch to include things as opposed to magic resolving.

Only think i'd potentially change is instead of

{% include 'hero' %}
{% include 'hero2' %}

It would be something like:

{% include 'hero' %}
{% include 'collectionshero' %}

I see "built" liquid files as slightly different to built CSS / JS. They can still somewhat be used to read and don't get obscured as much as JS / CSS so keeping it semi readable would be good imo

[tshamz]

What's the advantage of this over just modifying the original? What you're proposing sounds like Wordpress child themes, which existed to facilitate versioning the core/parent theme. Shopify themes don't really have anything similar to this, nor does Slate?

My main motivation here was to give devs in our organization a set of building blocks written in a way that prevents from making errors and comes prebuilt with functionality already baked in. The moment people start altering the core snippet library, bad things are liable to happen. Versioning the core snippets with something like a submodule was something I was also interested in as well. However, after reading you inputs and thinking about it, I might pivot on this a bit.

Anyways, as has been mentioned a few times before, I can get what I want without this PR so feel free to do whatever you'd like with it. Merge it, close it, ridicule it, worship it, whatever, I won't lose any sleep over it.

[t-kelly]

Versioning the core snippets with something like a submodule was something I was also interested in as well.

Sounds a little like #336