barrel / shopify-vite

Modern frontend tooling for Shopify theme development using Vite for a best-in-class DX.
https://shopify-vite.barrelny.com/
MIT License
293 stars 45 forks source link

Tailwind, theme editor reload, one save behind #137

Closed bigskillet closed 5 months ago

bigskillet commented 5 months ago

Tailwind is reloading nicely on the front, but it seems to be one save behind in the theme editor. I have --live-reload=full-page enabled. Am I missing something? Here are my package scripts:

"scripts": {
  "dev": "run-p -sr shopify:dev vite:dev",
  "shopify:dev": "shopify theme dev --store store-name --live-reload full-page",
  "vite:dev": "vite",
  "build": "vite build"
}
montalvomiguelo commented 5 months ago

The theme server instantly reflects local changes; for the theme editor, you could wait until the liquid changes synced to Shopify and then trigger the reload. The --notify flag + the page reload plugin can help here.

shopify theme dev --notify /tmp/theme.update

https://github.com/barrel/shopify-vite/blob/0490dc48f96e387784d0a4f8ad9638f79c5fbabc/examples/vite-shopify-example/vite.config.ts#L31-L33

bigskillet commented 5 months ago

Do I need to add a theme.update file to my repo? ...and is the delay required?

Also, I saw in a closed issue that Tailwind requires --live-reload full-page, but it seems to be working ok without it?

montalvomiguelo commented 5 months ago

You don't need to add a "theme.update" file to your repo. The delay is necessary in this case. After the file sync, we would wait a bit before triggering the reload; you can adjust the value as needed.

Skipping the --live-reload flag seems fine now 🎉

bigskillet commented 5 months ago

Remember to add --notify=/tmp/theme.update to your shopify theme dev script, y'all!

Ok, that works, but it's reloading twice – once on build, then it's triggered again by the plugin. I added --live-reload off to the Shopify script and hmr: false in the vite config, which seems to work. Is there another way?

montalvomiguelo commented 5 months ago

In this case, it makes sense to turn off the theme server's live reload and use the Vite one instead.

bigskillet commented 5 months ago

Turning off theme server's reload isn't enough. The hmr on the Vite server is still firing immediately, then the plugin is firing on the delay afterwards.

montalvomiguelo commented 5 months ago

Did you add a delay like 2000ms?

bigskillet commented 5 months ago

Yes

wturnerharris commented 5 months ago

@bigskillet - we're actually reviewing this live right now if you can jump in to show us what you're experiencing. Rare opportunity. Be here for the next 15 min: https://meet.google.com/vwm-kxjy-krj

bigskillet commented 5 months ago

Nice meeting you in the huddle!

To reiterate – I'm having to disable hmr in Vite and disable --live-reload on the theme server to get the plugin to fire independently. Here's my scripts:

"scripts": {
  "dev": "run-p -sr shopify:dev vite:dev",
  "shopify:dev": "shopify theme dev --store store-name --live-reload off  --notify=/tmp/theme.update",
  "vite:dev": "vite",
  "build": "vite build"
}
export default {
  server: {
    hmr: false
  },
  build: {
    emptyOutDir: false,
    rollupOptions: {
      output: {
        entryFileNames: '[name].[hash].min.js',
        chunkFileNames: '[name].[hash].min.js',
        assetFileNames: '[name].[hash].min[extname]',
      },
    }
  },
  plugins: [
    cleanup(),
    shopify({
      sourceCodeDir: "src",
      entrypointsDir: 'src/entrypoints',
      snippetFile: "assets.liquid"
    }),
    pageReload('/tmp/theme.update', {
      delay: 2000
    })
  ]
}
james0r commented 5 months ago

I just happened to be working on this today.

I'm noticing that with Vite version 5.3.1 i'm getting the manifest in the output dir no matter what I do.

  build: {
    manifest: false,
    emptyOutDir: false,
    rollupOptions: {
      output: {
        entryFileNames: '[name].[hash].min.js',
        chunkFileNames: '[name].[hash].min.js',
        assetFileNames: '[name].[hash].min[extname]',
      },
    }
  }

doesn't seem to work.

PageReload does seem to work okay as long as the delay is greater than 2000ms.

I added assets/.vite to my .shopifyignore so that I don't get the subfolders error.

As far as I can tell, I'm not using the manifest.

import shopify from 'vite-plugin-shopify'
import pageReload from 'vite-plugin-page-reload'
import basicSsl from '@vitejs/plugin-basic-ssl'
import { watch } from 'chokidar';
import fs from 'fs-extra'

const watchStaticAssets = () => ({
  name: 'watch-static-assets',
  configureServer(server) {
    const watcher = watch('./public/*', {
      persistent: true
    });

    const copyAsset = async path => {
      await fs.copy(path, `assets/${path.replace('public/', '')}`);
    }

    const removeAsset = async path => {
      await fs.remove(`assets/${path.replace('public/', '')}`);
    }

    watcher.on('add', copyAsset);
    watcher.on('change', copyAsset);
    watcher.on('unlink', removeAsset);
  }
})

export default {
  clearScreen: false,
  server: {
    host: '127.0.0.1',
    https: true,
    port: 3000,
    hmr: false
  },
  publicDir: 'public',
  plugins: [
    basicSsl(),
    watchStaticAssets(),
    shopify({
      sourceCodeDir: "src",
      entrypointsDir: 'src/entrypoints',
      snippetFile: "vite.liquid"
    }),
    pageReload('/tmp/theme.update', {
      delay: 2000
    })
  ],
  build: {
    manifest: false,
    emptyOutDir: false,
    rollupOptions: {
      output: {
        entryFileNames: '[name].[hash].min.js',
        chunkFileNames: '[name].[hash].min.js',
        assetFileNames: '[name].[hash].min[extname]',
      },
    }
  }
}
montalvomiguelo commented 5 months ago

Vite has to do a full refresh, as there are circular dependencies when using Tailwind. I found a workaround here; we could use the handleHotUpdate hook to filter out liquid modules and prevent a full refresh. With hmr enabled, let's try adding the following plugin to our Vite config file.

    pageReload('/tmp/theme.update', {
      delay: 2000
    }),
+   {
+     name: 'vite-plugin-liquid-tailwind-refresh',
+     handleHotUpdate(ctx) {
+       if (ctx.file.endsWith('.liquid')) {
+         // Filter out the liquid module to prevent a full refresh
+         return [...ctx.modules[0]?.importers ?? [], ...ctx.modules.slice(1)]
+       }
+     }
+   }

https://github.com/barrel/shopify-vite/assets/5134470/eda08fd8-5d81-491c-be89-e5f17980d730

james0r commented 5 months ago

Vite has to do a full refresh, as there are circular dependencies when using Tailwind. I found a workaround here; we could use the handleHotUpdate hook to filter out liquid modules and prevent a full refresh. With hmr enabled, let's try adding the following plugin to our Vite config file.

    pageReload('/tmp/theme.update', {
      delay: 2000
    }),
+   {
+     name: 'vite-plugin-liquid-tailwind-refresh',
+     handleHotUpdate(ctx) {
+       if (ctx.file.endsWith('.liquid')) {
+         // Filter out the liquid module to prevent a full refresh
+         return [...ctx.modules[0]?.importers ?? [], ...ctx.modules.slice(1)]
+       }
+     }
+   }

liquid-tailwind-prevent-refresh.mp4

I'm not really seeing the benefit of adding the custom plugin above. Still have to wait for the full round trip to see the updated styles.

Edit: I was using the wrong preview URL and that's why HMR wasn't working for sections. I've now got HMR for sections which is nice, but the above custom plugin doesn't seem to be preventing full reload after the 2000ms for liquid files.

montalvomiguelo commented 5 months ago

Hi @james0r, The second page reload is expected since we are using the --notify flag + pageReload plugin. The above plugin is to prevent Vite from performing the first refresh.

bigskillet commented 5 months ago

@montalvomiguelo, the vite-plugin-liquid-tailwind-refresh plugin is working great. Now that everything is refreshing properly, it's allowed me to reduce the page reload delay.