search

Shopify / slate

Slate is a toolkit for developing Shopify themes. It's designed to assist your workflow and speed up the process of developing, testing, and deploying themes.
https://shopify.github.io/slate
MIT License
1.28k stars 364 forks source link

Add the sections folder back to webpack context #1005

Closed dan-gamble closed 5 years ago

dan-gamble commented 5 years ago

What are you trying to accomplish with this PR?

Fix a small issue introduced by https://github.com/Shopify/slate/pull/1000.

With the new Slate Sections Plugin introduction, the sections folder got removed from Webpack's context. I believe it was previously added there by the CopyWebpackPlugin.

This PR adds it back into the context so making changes to the files causes the browser to reload as it used to.

Please provide a link to the associated GitHub issue. https://github.com/Shopify/slate/issues/1004

Checklist

For contributors:

dan-gamble commented 5 years ago

I signed the CLA but not quite sure how to re-run it.

harshal317 commented 5 years ago

Thanks a lot @dan-gamble, I made some styling changes so it would pass CI. I also got rid of the config import, as the sections path should be what we receive as the to option in the plugin, so figured it be best to just use that. I'll let @t-kelly take a look before publishing.

dan-gamble commented 5 years ago

Awesome. I figured there would be some changes as I'm not that au fait with the Shopify coding standard.

treechime commented 5 years ago

Thanks, @dan-gamble!

Can this be expected to be merged soon? The current version is very frustrating to develop with until this is fixed...

harshal317 commented 5 years ago

Thanks, @dan-gamble!

Can this be expected to be merged soon? The current version is very frustrating to develop with until this is fixed...

Sorry for the wait! Ideally we should get this merged today.

harshal317 commented 5 years ago

Should we be adding the files from this.options.from instead of this.options.to?

That was a mistake, ended up just adding to the context when we tap the emit hook, and added making sure the sections folder is in the context to the tests. 🎩 locally worked.

treechime commented 5 years ago

Perfect, thanks all 👍

lock[bot] commented 5 years ago

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.