Allow code to be organized in subfolders in `src/snippets` and `src/sections`

[t-kelly]

Developers like to keep their repos organized. One way of doing this is through folders and file groupings. This organizational method is possible in folders like src/styles and src/scripts but not possible in src/snippets and src/sections.

Let's allow developers to create whatever folder structure they want in the main src/ folders, and then output those folders in the correct structure for Shopify servers in the dist/ folder.

For example

src/
-- snippets/
---- snippet-1/
------ snippet-1a.liquid
------ snippet-1b.liquid
---- snippet-2/
------ snippet-2a.liquid
------ snippet-2b.liquid

would be transformed into the following on build:

dist/
-- snippets/
---- snippet-1a.liquid
---- snippet-1b.liquid
---- snippet-2a.liquid
---- snippet-2b.liquid

Related https://github.com/Shopify/slate/issues/271

[jonathanmoore]

I'd love to see this implemented for development. The only concern I have would be confusion if someone was using the same file name in more than one directory.

src/
-- snippets/
---- blog/
------ tags.liquid
---- collection/
------ tags.liquid

Would it make sense if the directory name were prepended to the file name as [directory].[filename].liquid? A quick test verified that {% include 'test.test' %} does correctly load /snippets/test.test.liquid

The example above would build as:

dist/
-- snippets/
------ blog.tags.liquid
------ collection.tags.liquid

[presto2116]

We are busily redesigning our entire site, and having this feature would be amazing. Our snippets and sections folders are starting to get very large. If we could break it off into folders the development experience will be a lot better. Please look into adding this feature.

[t-kelly]

@presto2116 This feature will be part of Slate v1. We are not adding any more features to Slate v0.

[NealEhardt]

Would it make sense if the directory name were prepended to the file name as [directory].[filename].liquid?

When I set up a directory structure, I expect to access it with slashes. So the most sensible way to consume a subfolder snippet would be {% include 'directory/filename' %}.

There's a conflict if you perform the proposed dot transform on a directory like this:

src/
-- snippets/
---- dogs/
------ fido.liquid
---- dogs.fido.liquid

Maybe as a prerequisite for this issue, Shopify's servers need to start accepting snippets subfolders.

[tshamz]

For anyone who comes across this in search of an answer to whether or not it's possible to include subdirectories in top level theme directories (e.g. snippets/ or sections/), I was able to get this working by altering my slate.config.js and extending the webpack config.

e.g.

const rules = [
  {
    test: /snippets\//,
    loader: 'file-loader',
    options: {
      name: `../snippets/[name].[ext]`,
    }
  },
];

module.exports = {
  slateTools: {
    webpackCommonExcludes: [
      'snippets/',
      'node_modules',
      'assets/static/',
    ],
    extends: {
      dev: { module: { rules } },
      prod:  { module: { rules } },
    },
  },
};

I haven't tested it out for directories other than src/snippets/, but I would hope it would work just the same. The major sticking point here for me was needing to include the webpackCommonExcludes option in order to have slate-tools core.js loaders ignore the theme directories with subdirectories in them and instead only use my extension to the webpack config. The thing that's not instantly apparent here is that initially I only included webpackCommonExcludes: ['snippets/'], which created a bunch of babel-loader issues for me. After discovering #705, I realized I needed to also add node_modules and assets/static/ to my ignore since they are the default values used and instead of augmented with my values, they're completely blown out.

I very briefly tested how webpack handles namespace collisions and it seemed like it was consistent with how it chose which file to place into dist/, although I did run into some wonky behavior when removing files but having them persist through. I have no idea exactly how it decides which file wins, but my major observation was that files more deeply nested won out over files in the same path as them e.g. snippets/group1/group2/test.liquid beat snippets/group1/test.liquid and snippets/test.liquid. Maybe someone has some insight into exactly how webpack is making this decision?

[t-kelly]

@tshamz looks like you could make a PR? 😄

[RustyDev]

const rules = [
  {
    test: /snippets\//,
    loader: 'file-loader',
    options: {
      name: `../snippets/[name].[ext]`,
    }
  },
];

module.exports = {
  slateTools: {
    webpackCommonExcludes: [
      'snippets/',
      'node_modules',
      'assets/static/',
    ],
    extends: {
      dev: { module: { rules } },
      prod:  { module: { rules } },
    },
  },
};

@tshamz Are you able to yarn deploy to production with that code? I'm trying and it's throwing errors:

Status: 422 Unprocessable Entity
Errors: Theme files may not be stored in subfolders

[tshamz]

yan, I'm able to deploy to the live published theme using yarn deploy, however I'm using the more recent version after breaking changes were introduced to slate.config.js (beta.8 or 9 I think?) so my config file looks a little different than yours now (don't mind all the other plugins and aliases):

/* eslint-disable */

// Configuration file for all things Slate.
// For more information, visit https://github.com/Shopify/slate/wiki/Slate-Configuration

const path = require('path');
const { ProvidePlugin } = require('webpack');

const plugins = [
  new ProvidePlugin({
    '$': 'jquery',
    'jQuery': 'jquery',
    'window.jQuery': 'jquery',
    'PubSub': 'pubsub-js',
  }),
];

const alias = {
  'jquery': path.resolve('./node_modules/jquery'),
  'lodash-es': path.resolve('./node_modules/lodash-es'),
  'slick': path.resolve('./node_modules/slick-carousel'),
  'styles': path.resolve('./src/assets/styles'),
  'scripts': path.resolve('./src/assets/scripts'),
  'core': path.resolve('./src/assets/scripts/core'),
  'bindings': path.resolve('./src/assets/scripts/bindings'),
  'containers': path.resolve('./src/assets/scripts/containers'),
  'handlers': path.resolve('./src/assets/scripts/handlers'),
  'nodes': path.resolve('./src/assets/scripts/nodes'),
  'controls': path.resolve('./src/assets/scripts/controls'),
};

const rules = [
  {
    test: require.resolve('jquery'),
    use: [{
      loader: 'expose-loader',
      options: '$'
    }]
  },
  {
    test: /snippets\/.*\.liquid$/,
    loader: 'file-loader',
    options: {
      name: `../snippets/[name].[ext]`,
    }
  },
  {
    test: /sections\/.*\.liquid$/,
    loader: 'file-loader',
    options: {
      name: `../sections/[name].[ext]`,
    }
  },
];

module.exports = {
  'cssVarLoader.liquidPath': [
    'src/snippets/base/css-variables.liquid'
  ],
  'webpack.extend': {
    plugins,
    resolve: { alias },
    module: { rules },
  },
  'webpack.commonExcludes': [
    /node_modules/,
    /assets\/static/,
    /sections\/.*\.liquid$/,
    /snippets\/.*\.liquid$/,
  ],
};

[RustyDev]

Strange, I still get the same error after upgrading all packages and using your config (without plugins).

[adamwooding]

Hi @tshamz , thank you for your configuration file. I'm trying to get this to work in Slate version 1.0.0-beta.12 so that I can have folders inside my snippets directory. I have installed the Webpack file-loader dependency & updated my Slate config, but Slate doesn't flatten the directory on output (just copies the subdirectories). Here's my config file:

/* eslint-disable */

// Configuration file for all things Slate.
// For more information, visit https://github.com/Shopify/slate/wiki/Slate-Configuration

const path = require('path');

const alias = {
  'jquery': path.resolve('./node_modules/jquery'),
  'lodash-es': path.resolve('./node_modules/lodash-es')
};

const rules = [
  {
    test: /snippets\/.*\.liquid$/,
    loader: 'file-loader',
    options: {
      name: `../snippets/[name].[ext]`,
    }
  },
  {
    test: /sections\/.*\.liquid$/,
    loader: 'file-loader',
    options: {
      name: `../sections/[name].[ext]`,
    }
  },
];

module.exports = {
  'cssVarLoader.liquidPath': [
    'src/snippets/css-variables.liquid'
  ],
  'webpack.extend': {
    resolve: {alias},
    module: {rules}
  },
  'webpack.commonExcludes': [
    /node_modules/,
    /assets\/static/,
    /sections\/.*\.liquid$/,
    /snippets\/.*\.liquid$/,
  ],
};

Would you mind letting me know what i'm doing wrong please? Thank you so much, Adam

[RustyDev]

I created a new slate project using the config above to illustrate the issue. There's an app subdirectory under snippets with a file in it. After running yarn build, the subdirectory isn't flattened in dist/snippets:

https://github.com/RustyDev/slate-snippet-folder-test

https://github.com/RustyDev/slate-snippet-folder-test/blob/master/slate.config.js

https://github.com/RustyDev/slate-snippet-folder-test/tree/master/src/snippets/app

The only way I've been able to get this to work is by adding flatten: true in @shopify/slate-tools tools/webpack/config/parts/core.js:109:

{
  from: config.get('paths.theme.src.snippets'),
  to: config.get('paths.theme.dist.snippets'),
  flatten: true,
},

[tshamz]

hmmmmmm yah, something must have changed in one of the recent updates. Everything works for me running slate-tools v1.0.0-beta.9, but if I create a new slate project running slate-tools v1.0.0-beta.14 it appears as if my config no longer works.

tbh, webpack is voodoo magic to me. I don't have a very good grasp of what's going on behind the scenes. if you guys figure something out, report back in.

[tshamz]

@rustydev, I bet you could actually accomplish adding that flatten attribute via the slate config file instead of editing the module’s code, I’m just not sure how exactly. Maybe I’ll noodle on it for a bit and report back in.

[t-kelly]

You should be able to add the following to your slate.config.js for this to work:

const CopyWebpackPlugin = require('copy-webpack-plugin')

module.exports = {
  plugins: [
    new CopyWebpackPlugin(
      { from: 'src/snippets/*', to: 'dist/snippets', flatten: true },
      { from: 'src/sections/*', to: 'dist/sections', flatten: true }
    )
  ]
}

[RustyDev]

I tried this (added the brackets since it's an array)

  plugins: [
    new CopyWebpackPlugin([
      { from: 'src/snippets/*', to: 'dist/snippets', flatten: true },
      { from: 'src/sections/*', to: 'dist/sections', flatten: true }
    ])
  ]

Doesn't seem to be working though:

Status: 422 Unprocessable Entity
Errors: Theme files may not be stored in subfolders

Here's my config:

const path = require('path');
const CopyWebpackPlugin = require('copy-webpack-plugin')

module.exports = {
  'cssVarLoader.liquidPath': [
    'src/snippets/css-variables.liquid'
  ],
  'webpack.extend': {
    resolve: { 
      alias: {
        'jquery': path.resolve('./node_modules/jquery'),
        'lodash-es': path.resolve('./node_modules/lodash-es'),
      }
    },
    plugins: [
      new CopyWebpackPlugin([
        { from: 'src/snippets/*', to: 'dist/snippets', flatten: true },
        { from: 'src/sections/*', to: 'dist/sections', flatten: true }
      ])
    ]
  },
};

[tshamz]

@RustyDev try updating the paths to this...

new CopyWebpackPlugin([
    { from: 'snippets/**/*', to: '../snippets/', flatten: true },
    { from: 'sections/**/*', to: '../sections/', flatten: true },
]),

This will work, but anytime you update a file in one of your snippets/sections subdirectories, the watcher will upload two files. One of the uploads will succeed, and one will fail since the CopyWebpackPlugin from core.js is still trying to upload the unflattened file. Idk how to go about disabling the patterns for snippets/ and sections/ from the core.js instance of CopyWebpackPlugin, but if you can figure that out you should be good to go.

[RustyDev]

Thanks, @tshamz. That was a silly error on my part. I'll give this a go.

I'm wondering at this point if it makes sense to open a pull request with that change to @shopify/slate-tools/tools/webpack/config/parts/core.js. Is there a reason that's not advisable? Seems like the easiest solution for this issue. After having everything organized in folders for a few weeks, it's made my codebase much easier to maintain. I'm sure others would feel the same.

[tshamz]

Idk if a pull request would get approved, but it’s worth a shot (especially since it’s such a tiny change codewise).

The reasons I think a PR might be met with some resistance are:

1) the way CopyWebpackPlugin resolves namespace collisions is non-deterministic and for some people that’s not ideal

2) based on what I can tell from digging into the slate-tools codebase and working with slate themes for a while now, it seems like everything has been architected so that developers can customize their slate projects via slate.config.js instead of needing to change the underlying codebase.

[tshamz]

after revisiting this briefly, I realized a quick and dirty workaround for this is to change the path for src/snippets/ in slate.config.js to either an empty directory or snippet subdirectory that you know won't have any further subdirectories and then adding an ignore property to the CopyWebpackPlugin added to the extended webpack config.

const CopyWebpackPlugin = require('copy-webpack-plugin')

const plugins = [
  new CopyWebpackPlugin([
      {
        from: 'snippets/**/*',
        to: '../snippets/',
        flatten: true,
        ignore: [ 'icons/*.liquid' ],
      },
  ]),
];

module.exports = {
  'cssVarLoader.liquidPath': ['src/snippets/css-variables.liquid'],
  'paths.theme.src.snippets': 'snippets/icons',
  'webpack.extend': {
    plugins,
  },
};

and src/snippets/ can look like: without any errors or double uploading.

It's not very elegant or pretty, but it gets the job done and allows for subdirectories in src/snippets/ and src/sections/ again without any errors or odd behavior. If anyone figures out something cleaner, sound off.

[alexcasche]

I was able to remove the subfolders after webpack build by adding this to my plugins. Requires copy-webpack-plugin on-build-webpack and rimraf

new CopyWebpackPlugin([
    {
        from: 'sections/**/*',
        to: '../sections/',
        flatten: true
    },
   {
        from: 'snippets/**/*',
        to: '../snippets/',
        flatten: true
    }
]),
new WebpackOnBuildPlugin(function(stats) {
    rimraf("./dist/sections/!(*.liquid)", function () { 
        console.log("removed section subfolders"); 
    });
     rimraf("./dist/snippets/!(*.liquid)", function () { 
         console.log("removed snippets subfolders"); 
     });
})

[ajhaupt7]

^ I had the above working, but this approach seems to be broken after the introduction of the Slate Sections Plugin in https://github.com/Shopify/slate/pull/982/files#diff-59541ead0c8a4e18bc4cdcfda93131d5

[cjayyy]

@presto2116 This feature will be part of Slate v1. We are not adding any more features to Slate v0.

I see you already implemented "slate-sections-plugin" in v1. The problem described here is regarding snippets as well. Does this plugin work with snippets too? If not, can you please explain, why?

Thanks in advance, great app!

[phpMagpie]

Hi all, what is the state of play with this enhancement?

Working on our first slate theme and would be amazing if we could create subfolders for sections used in custom templates.