[WIP] Slate Sections Plugin

[harshal317]

What are you trying to accomplish with this PR?

In order to support a sections format that allows separating the schema from the liquid, and allowing a structure that makes it easy to maintain translations for many languages, @t-kelly and I came to a conclusion it may be a good idea to move away from CopyWebpackPlugin. Especially keeping in mind the potential for additional features to this plugin.

This plugin aims to support three basic structures

1. Having a sections folder consisting of simply liquid files, which will simply be copied to the dist sections path

2. Having a sections folder consisting of folders for each section following the naming convention /sections/example-section-name/ having a mandatory 'template.liquid' file, and an optional 'schema.json' file. If there is a schema file in the directory it will get appended to the liquid file adding the schema tags, and will be outputted to the dist sections path taking the name of the directory (example-section-name.liquid)

3. Similiar to option 2, however it extends the functionality by adding a locales folder which can have multiple json files for each language. The translations will automatically be added to the json object that gets appended to the liquid making it easier to maintain multiple languages and keeping a cleaner codebase

To Do

• Adding more robust tests,
• Adding a README
• Taking in and improving the plugin based on feedback
• Adding error handling

Checklist

For contributors:

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

For maintainers:

• I have 🎩'd these changes.

[harshal317]

The dirent class was added in Node 10, so might have to change that logic slightly to support older versions.

[t-kelly]

• Please update the PR description to include a description of the problem you're solving and summary of what the plugin is doing.
• Can we rename the plugin to slate-sections-plugin? Will allow us to add more functionality to the sections build in the future.
• You're doing a lot of file reading and writing here -- lets make sure we use async methods for concurrent operations.
• Include another level of nesting to make sure we're crawling through all nested layers?
• packages/slate-locales-plugin/__tests__/compiler-tests.js -> packages/slate-locales-plugin/__tests__/index.test.js
• You could simplify the contents your fixtures, especially the Liquid. They should the minimum amount of code to pass your tests -- having them filled with extra stuff will make them harder to maintain

[t-kelly]

From @davidwarrington:

Would it be possible to add .js file support to this? Being able to import a schema exported from a JS file would open up a lot of possibilities. For example; querying an API to retrieve a list of options, or simply being able to build multiple similar options with a for loop. Most stores probably don't require anything like this however I have worked on a few stores where this would be beneficial.

The way I see this working is by using const jsonSchema =require(jsonSchemaFilePath) in line 22 of inject-schemas-to-sections.js. Since Node caches modules, they would need to be flushed out of the cache when updating, in order to be re-imported, but that's not a particularly difficult job.

[harshal317]

From @davidwarrington:

Would it be possible to add .js file support to this? Being able to import a schema exported from a JS file would open up a lot of possibilities. For example; querying an API to retrieve a list of options, or simply being able to build multiple similar options with a for loop. Most stores probably don't require anything like this however I have worked on a few stores where this would be beneficial.

The way I see this working is by using const jsonSchema =require(jsonSchemaFilePath) in line 22 of inject-schemas-to-sections.js. Since Node caches modules, they would need to be flushed out of the cache when updating, in order to be re-imported, but that's not a particularly difficult job.

This is definitely something that can be done, however it is probably worth discussing whether we want to do it in this current PR or as a feature moving forward.

[harshal317]

This looks great! @t-kelly made my life easier with his early reviews (thanks). I still need to 🎩but I'll do that first thing tomorrow morning.

Side note: did you and @t-kelly discuss the possibility of migrating the locale logic over so we can use it for merchant-facing translations for config/settings_schema.json, not just for sections? This would enable a similar folder structure to how we're structuring section folders and enable translations to be referencing with the t key:

./config
├── settings_schema.json
├── locales/
   ├── en.json
   ├── fr.json
   ├── ...

We haven't discussed that yet, however if it would be helpful to do so, I don't see why not. Could probably consider doing that through the CopyWebpackPlugin's transform as well.

[harshal317]

Since folders are looking to be introduced here would it be much extra effort to allow an additional layer of folders in the sections folder? For example:

./sections
├── homepage/
   ├── featured-collections/
      ├── schema.json
      ├── template.liquid
   ├── hero/
      ├── schema.json
      ├── template.liquid
   ├── ...
├── product/
   ├── featured-articles/
      ├── schema.json
      ├── template.liquid
   ├── variants/
      ├── schema.json
      ├── template.liquid
   ├── ...

I believe we discussed having a deeper folder structure, and decided that it could be added as a feature moving forward.

[bertiful]

We haven't discussed that yet, however if it would be helpful to do so, I don't see why not.

It's definitely helpful, and it reflects how we're already approaching merchant-facing translations (this is how locales are currently formatted in our themes for both the ./sections and ./config folders).

[bertiful]

🎩'd and works oh so smoothly :) good job

[dan-gamble]

Didn't think this would be in so quick. Can't wait for 0.16 😍

[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.