Skip to content
This repository has been archived by the owner on Jul 19, 2021. It is now read-only.

Simplify Slate assets #850

Merged
merged 1 commit into from
Oct 25, 2018
Merged

Simplify Slate assets #850

merged 1 commit into from
Oct 25, 2018

Conversation

t-kelly
Copy link
Contributor

@t-kelly t-kelly commented Oct 22, 2018

TL;DR: Slate performs a number of magic transformations to the theme’s src/assets/ directory which ultimately result in developer confusion. This PR simplifies how Slate manages and transforms assets to achieve an easier to understand and reliable developer experience.

Problems

The /assets directory

In its current state, Slate does a lot of magic to a theme’s assets/ directory, including:

  • Copying files from assets/svgs to snippets/ so SVGs can be inlined in a theme using {% include %}
  • Transforming the contents of assets/scripts into layout and template specific bundles.
  • Copying files from src/assets/static to dist/assets
  • Copying files from src/assets/images to dist/assets
  • Only copies other files from src/assets if they are referenced using the asset_url somewhere in a Liquid file ({{ '../assets/my-image.jpg' | asset_url }}), or imported into one of the bundles (import '../styles/theme.scss')
Related Issues

Referencing assets via relative paths

Slate also attempts to serve assets from a local asset server by using relative paths: ``` {{ '../assets/my-image.jpg' | asset_url }} ```

And then swaps the path for one that references the dev machine's local IP when in development, or one that will result in a valid cdn.shopify.com path for production build. This works great when the tag is structured exactly like the above, but breaks for examples like the following:

  1. Path is contained in a variable:
{% assign myPath = '../assets/my-image.jpg' %}
{{ myPath | asset_url }}
  1. More than one filter is used:
{{ '../assets/my-image.jpg' | asset_url | img_tag }}
Related issues

Solution

This PR includes the following changes:

  • All files inside src/assets are statically copied to dist/assets. Folders inside src/assets are not copied. Contents of folders are copied. We need to flush out what happens when there is a naming conflict. Using flatten option from copy-webpack-plugin
  • All assets are referenced by filename only, as with native Shopify themes, e.g. {{ '../assets/my-image.jpg | asset_url }} is now {{ 'my-image.jpg' | asset_url }}.
  • Only JS and styles referenced in snippets/style-tags.liquid and snippets/script-tags.liquid are fetched from your local server in development, all other assets referenced in your theme are fetched from Shopify CDN. This makes sure we can still take advantage of Hot Module Replacement and quick change previewing while modifying styles and scripts.
  • The contents of src/assets/svgs should be renamed from `
  • The contents of src/assets/scripts have moved to src/scripts
  • The contents of src/assets/styles have moved to src/styles

TODO

  • Full page reload always occurs after syncing changed files. We shouldn't do this is HMR was successful
  • Open PR with updates to Starter Theme Simplify assets starter-theme#121
  • Open PR with updates to Skeleton Theme Simplify assets skeleton-theme#9
  • Update docs
  • Deprecate slate-liquid-asset-tags-plugin on NPM
  • Deprecate slate-liquid-asset-loader on NPM

@t-kelly t-kelly changed the title (WIP) Simplify Slate assets Simplify Slate assets Oct 25, 2018
@t-kelly t-kelly mentioned this pull request Oct 30, 2018
@lmartins lmartins mentioned this pull request Nov 1, 2018
@lock
Copy link

lock bot commented Dec 2, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant