Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watch doesn't trigger while processing scss with Gulp #607

Open
AdamDSherman opened this issue Jan 14, 2019 · 26 comments
Open

Watch doesn't trigger while processing scss with Gulp #607

AdamDSherman opened this issue Jan 14, 2019 · 26 comments

Comments

@AdamDSherman
Copy link

AdamDSherman commented Jan 14, 2019

Describe the bug
I am using Gulp to compile several .scss files into my main theme.scss.css file (which is in the assets directory). This works fine, but Theme Watch doesn't recognise when the theme.scss.css file has been updated when recompiled. However, if I manually update the theme.scss.css file in my IDE it will recognise the change and push up to the remote server.

Working with other normal section, asset and snippet files works fine - theme watch detects the change and pushes them up.

To Reproduce
Steps to reproduce the behavior:

  1. Install Gulp and create the following Gulpfile.js:
const gulp = require('gulp');
const sass = require('gulp-sass');
const rename = require("gulp-rename");

gulp.task('sass', function(){
  return gulp.src('scss/main.scss')
	.pipe(sass())
	.pipe(gulp.dest('css'))
});

gulp.task('rename', function(){
return gulp.src("css/main.css")
  .pipe(rename("theme.scss.css"))
  .pipe(gulp.dest("assets"));
});

gulp.task('do-sass', gulp.series('sass', 'rename'))`
  1. Run theme watch
  2. Compile scss with the gulp do-sass command
  3. theme watch may push up the first set of changes to theme.scss.css, but not any subsequent changes

Expected behavior
A clear and concise description of what you expected to happen.

Environment (please complete the following information):

  • OS [e.g. iOS]: MacOS 10
  • Themekit version [e.g. v0.7.5]: 1.0.0
  • Editor [e.g. atom, sublime]: Sublime

Additional context
Add any other context about the problem here.

@tanema
Copy link
Contributor

tanema commented Jan 14, 2019

Hey thanks for all the details, I am looking into debugging but my main question is why you are compiling scss to css and then outputting it to the assets folder with the .scss.css extension? Why not just have these files in your assets folder with the .scss.css extension and let shopify take care of your scss compilation?

@tanema
Copy link
Contributor

tanema commented Jan 14, 2019

Okay so I have not been able to reproduce this even with your gulp script and your setup. The only way I have been able to reproduce the behaviour is by running the do-sass task when nothing has changed. The newest watch backend uses checksums to ensure to only enqueue tasks if the file has changed it's contents. Is it possible that your task is running without having any actual code changes?

@AdamDSherman
Copy link
Author

AdamDSherman commented Jan 14, 2019

Hey thanks for all the details, I am looking into debugging but my main question is why you are compiling scss to css and then outputting it to the assets folder with the .scss.css extension? Why not just have these files in your assets folder with the .scss.css extension and let shopify take care of your scss compilation?

Hi Tanema, many thanks for looking into this.

I might be wrong (I am still very new to Shopify and ThemeKit) but I don't think Shopify likes SASS's @import.

I have all my colour variables on their own colours.scss file, same with typography.scss, utils.scss, buttons.scss etc. They all contain variables and mixins I use on other .scss files. If I had all these in the assets directory wouldn't Shopify just compile them all as individual .css files?

However I do think my gulpfile.js could be streamlined - there's probably no need for rename. I could just compile the scss straight to assets/theme.scss.css instead of copying with rename.

@AdamDSherman
Copy link
Author

AdamDSherman commented Jan 14, 2019

Okay so I have not been able to reproduce this even with your gulp script and your setup. The only way I have been able to reproduce the behaviour is by running the do-sass task when nothing has changed. The newest watch backend uses checksums to ensure to only enqueue tasks if the file has changed it's contents. Is it possible that your task is running without having any actual code changes?

Hmm strange... perhaps it's a caching thing on Terminal or my environment? Yes, I have tested to make sure and there definitely are changes on the compiled file (the way I've been uploading quick css fixes to my site is I've compiled the .css file with my changes, then manually added a line-break to the bottom of the .css file to trigger theme watch and upload them).

Weirdly enough too, theme watch does pick up the changes on the very first compile, but not subsequent compiles after that. When I have a chance I'll see if I can attach a gif of what I see. Have you tried a few compiles one after the other?

Many thanks for looking into this again.

@trev-dev
Copy link

trev-dev commented Jan 16, 2019

I am able to reproduce this challenge with the following gulpfile:
(gulp v4)

const { task, watch, src, dest, series } = require('gulp');
const sass = require('gulp-sass');
const autoprefix = require('gulp-autoprefixer');

sass.compiler = require('node-sass');

function scss(cb) {
  src('./src/scss/**/*.scss')
    .pipe(sass({outputStyle: 'expanded'}))
    .on('error', sass.logError)
    .pipe(autoprefix({
      browsers: ['> 2%', 'ie >= 10'],
      grid: true
    }))
    .pipe(dest('./assets/'));

    cb();
}

function watcher(cb) {
  watch('./src/scss/**/*.scss', series(scss))
  cb()
}

module.default = task('default', series([scss, watcher]));

File Structure:

.
├── assets
├── config
├── config.yml
├── gulpfile.js
├── layout
├── locales
├── package.json
├── package-lock.json
├── readme.md
├── readme.org
├── snippets
├── src
│   └── scss
│       ├── custom.scss
│       └── partials
│           ├── _construction_sale.scss
│           ├── _exports.scss
│           ├── _flex_containers.scss
│           ├── _footer.scss
│           ├── _grid_blog.scss
│           ├── _header.scss
│           ├── _ikidz.scss
│           └── _navigation.scss
└── templates

The problem didn't happen prior to the 1.0.0 update.

Funny enough, I can create a newline in the compiled css file, delete the same line, save it and Shopify Theme Kit will detect that and subsequent compiled changes for 2-3 more changes before it starts ignoring the compiled output again.

Like @AdamDSherman, I have several partial imports, including a _variables.scss file. I'm not sure that Shopify handles autoprefixing in addition to imports, so I would prefer that this tool watch consistently as well. I'd contribute something more substantial but I don't speak go (yet?) Thanks so much!!

@AdamDSherman
Copy link
Author

AdamDSherman commented Jan 16, 2019

Just some more info if it's useful:

I recently have updated my above gulpfile.js to use gulp watch with my .scss files, running the do-sass task. Since doing this change, like @trev-dev is experiencing, ThemeKit detects 2-3 subsequent changes, but none beyond there. Very odd.

@tanema
Copy link
Contributor

tanema commented Jan 16, 2019

Thank you for all your information, I still cannot reproduce it for some reason. So here we go with a full deep dive:

Here is what I am using

Gulp version 4.0
npm version 6.4.1
node version v8.12.0

package.json

{
  "scripts": {
    "watch": "gulp"
  },
  "devDependencies": {
    "gulp": "^4.0.0",
    "gulp-autoprefixer": "^6.0.0",
    "gulp-rename": "^1.4.0",
    "gulp-sass": "^4.0.2"
  }
}

Gulpfile.js
Same exact file as @trev-dev

What I am doing

  • I have a src/scss/main.scss that imports src/scss/partials/_variables.scss and compiles out to assets/main.css.
  • I am running theme watch in one console and npm run watch which runs gulp in another console. (Are you used foreman or something like that to run both tasks at the same time?)
  • Changing both main.scss and _variables.scss

All of the changes are picked up by uploading assets/main.css using both sublime text and vim. Is there something I am missing at all in your setup or at least version difference in any of the tools here?

Lastly I appreciate your help and patience while we get this fixed!

@trev-dev
Copy link

@tanema - I'm not running any sort of task runner, or even npm, outside of gulp itself. I just head over to my project root and type shopifytk watch (I renamed the binary) and in a seperate session, run gulp from the project root.

I'm fairly sure this isn't related, but just in case it is, I did not run the themekit install script. I just read the script over and determined which binary to download and add to my local bin path that's exported in my .zshenv. I use Arch, btw (memes aside).

@tanema
Copy link
Contributor

tanema commented Jan 16, 2019

@trev-dev
Re: Install Did you do all this manual work because the install script does not work for you? I imagine that might be the case since I do not have an entry for your arch. If you could give me the output of echo "$(uname) $(uname -m)" I will be able to support other Arch users in the future.

Re: Usage I was actually able to reproduce this now but only for my _variables.scss partial but if I save my main.scss file then themekit picks up the changes! I will get back to you with something soon hopefully.

@trev-dev
Copy link

@tanema I was inspecting the install script to see how it worked (I try to sandbox as much software as I much as I possibly can to my local user) and figured out where the binary comes from and how it's run and just went "Oh, this is straight forward" and downloaded it myself :) - I didn't even try to run the script. Thanks for sticking with us.

@trev-dev
Copy link

@trev-dev
echo "$(uname) $(uname -m)" I will be able to support other Arch users in the future.

I beg your pardon @tanema - here's the output for this: Linux x86_64

@tanema
Copy link
Contributor

tanema commented Jan 16, 2019

Okay so I have noticed that when I change main.scss (the file that is outputted directly as assets/main.css) the timestamp changes but when I change my variables the timestamp does not change on the file. You can explore this yourself by running date -r main.css on your own file.

The newest implementation uses polling based on checksums and timestamps and no longer uses inotify for linux and kqueue for OSX. (library for reference) This means that if your tool, gulp in this instance does not change the mod time on a file themekit won't pick it up. I will look into a way we can make this better through the library. I believe what it does currently is first check the time and then check the checksum but maybe we can make it do both.

For now as a workaround I found a gulp lib called gulp-touch that would touch the file for every modification and made this change to my gulpfile.js

const { task, watch, src, dest, series } = require('gulp');
const sass = require('gulp-sass');
const autoprefix = require('gulp-autoprefixer');
const touch = require('gulp-touch'); //added this

sass.compiler = require('node-sass');

function scss(cb) {
  src('./src/scss/**/*.scss')
    .pipe(sass({outputStyle: 'expanded'}))
    .on('error', sass.logError)
    .pipe(autoprefix({
      browsers: ['> 2%', 'ie >= 10'],
      grid: true
    }))
    .pipe(dest('./assets/'))
    .pipe(touch()); //added this

    cb();
}

function watcher(cb) {
  watch('./src/scss/**/*.scss', series(scss))
  cb()
}

module.default = task('default', series([scss, watcher]));

I hope this speeds up your development again and I will hopefully have a more official fix for you in the future.

@richyrb00
Copy link

richyrb00 commented Jan 16, 2019

We seem to be getting a similar issue.

Themekit 0.8.1

When we update an existing class it updates the assets file just fine. when we add a new class it doesn't. the main.min.css it compiles into has the updated code it just isn't then sending it over to the shopify theme .

note this only happens on watch but we can build and deploy manual fine.

@trev-dev
Copy link

Okay so I have noticed that when I change main.scss (the file that is outputted directly as assets/main.css) the timestamp changes but when I change my variables the timestamp does not change on the file. You can explore this yourself by running date -r main.css on your own file.

I didn't even realize that you could modify a file without implicitly modifying the timestamp. I wonder if this is something the folks at Gulp would find handy.

@trev-dev
Copy link

trev-dev commented Jan 16, 2019

Of course, I went and had a look and found gulpjs/gulp#2193 @tanema

...so it appears its a gulp issue first and a Shopify Themekit issue second.

@tanema
Copy link
Contributor

tanema commented Jan 16, 2019

Currently, this is a feature of gulp 4 due to a limitation of node's fs.Stat object. If you need mtime to change - you need to modify the mtime on the files in your pipeline.

So it seems like will not be fixed in Gulp, we will have to find ways of handling it downstream and it sounds like they even suggest something like my gulp-touch solution. I will see if there is something I can do within themekit to handle this better without effecting other tools

Edit: It seems like even in the other issues linked in the gulp issue, other developers have solved this in a similar way.

@trev-dev
Copy link

trev-dev commented Jan 16, 2019

@tanema Love the accountability for your user experience. For what it's worth, Gulp is sorta looking into it too, it's been offloaded into another issue but it has been open for nearly 2 years now around 3 years now.

gulpjs/vinyl#105

@tanema
Copy link
Contributor

tanema commented Jan 16, 2019

As a note for the future as @richyrb00 has mentioned that older versions are broken, this is not a change in themekit that caused this but rather a change in Gulp and/or Node. I recommend using touch-gulp as a current workaround and also to keep older verisons of themekit working with your gulp workflow if a fix is deployed for this.

@tanema tanema changed the title Theme Watch doesn't trigger with compiled scss files Watch doesn't trigger while processing scss with Gulp Jan 16, 2019
@trev-dev
Copy link

Currently, this is a feature of gulp 4 due to a limitation of node's fs.Stat object. If you need mtime to change - you need to modify the mtime on the files in your pipeline.

So it seems like will not be fixed in Gulp, we will have to find ways of handling it downstream and it sounds like they even suggest something like my gulp-touch solution. I will see if there is something I can do within themekit to handle this better without effecting other tools

Edit: It seems like even in the other issues linked in the gulp issue, other developers have solved this in a similar way.

gulp-touch (and similar gulp-touch-xxx) npm packages are 2 years out of date and don't appear to be working for gulp 4, FYI. I was able to achieve results using through2 which will accept a Writeable stream and allow me to change the atime/mtime.

const { task, watch, src, dest, series } = require('gulp');
const sass = require('gulp-sass');
const autoprefix = require('gulp-autoprefixer');
const t2 = require('through2'); // Get through2 as t2

sass.compiler = require('node-sass');

function scss(cb) {
  src('./src/scss/**/*.scss')
    .pipe(sass({outputStyle: 'expanded'}))
    .on('error', sass.logError)
    .pipe(autoprefix({
      browsers: ['> 2%', 'ie >= 10'],
      grid: true
    }))
    .pipe(t2.obj((chunk, enc, cb) => { // Execute through2
      let date = new Date();
      chunk.stat.atime = date;
      chunk.stat.mtime = date;
      cb(null, chunk);
    }))
    .pipe(dest('./assets/'))

    cb();
}

function watcher(cb) {
  watch('./src/scss/**/*.scss', series(scss))
  cb()
}

module.default = task('default', series([scss, watcher]));

This is working like a charm for me.

@tanema
Copy link
Contributor

tanema commented Jan 17, 2019

@trev-dev thank you so much for the update I am sure others will appreciate it!

@philippeschneide
Copy link

Thanks trev-dev, you rock...

@Kehza
Copy link

Kehza commented Mar 24, 2020

I couldn't get Trev-Dev's solution working for me, so came up with an alternative. Here's a stripped-down version. I essentially run a seperate Gulp Watch on the generated CSS file, and perform a 'deploy' on that specific file.

const themeKit = require('@shopify/themekit');
function uploadStyle(){
    return themeKit.command('deploy', {
        config: params.config_live_yml,
        files: [params.styles.dest+'/styles-theme.min.css.liquid'],
        env: "live"
    }).catch(err => console.error('Error', err)); 
}

function processStyles(){
    return gulp.src(params.styles.src)
        .pipe(autoprefixer({
          browsers: ['last 2 versions'],
          cascade: false
        }))
        .pipe(cleanCSS())
        .pipe(rename({
          basename: 'styles-theme',
          suffix: '.min',
          extname: '.css.liquid'
        }))
        .pipe(gulp.dest(params.styles.dest));
}

function shopify(){
    gulp.watch(params.styles.watch, processStyles);
    gulp.watch(params.styles.dest+'/styles-theme.min.css.liquid', uploadStyle);

    themeKit.command('watch', {
        config: params.config_live_yml,
        notify: 'notify.tmp',
        env: "live"
    }).catch(err => console.error('Error', err));  
}

gulp.task('default', function() {
  shopify();
});

@rickydazla
Copy link

I just ran across this.. As @tanema notes above, it seems to because the timestamp does not change. There are a few options, like gulp-touch (or gulp-touch-fd better, for use with Gulp 4). I liked this one from stackoverflow:

.pipe(through2.obj(function(file,enc,cb){ // change mod date, for qs watch
  let date = new Date();
  file.stat.atime = date;
  file.stat.mtime = date;
  cb(null,file);
}))

@WraithKenny
Copy link

WraithKenny commented Apr 22, 2020

@rickydazla and @trev-dev I'm glad you found my little pipe function useful!

I've updated it to be better and more reusable:

import through2 from 'through2';
const touch = () => through2.obj( function( file, enc, cb ) {
  if ( file.stat ) {
    file.stat.atime = file.stat.mtime = file.stat.ctime = new Date();
  }
  cb( null, file );
});

So you can use .pipe( touch() ) like so

function sassDev() {
  return gulp.src( sassFiles )
    .pipe( sourcemaps.init() )
    .pipe( sass() )
    .pipe( postcss() )
    .pipe( sourcemaps.write() )
    .pipe( touch() )
    .pipe( gulp.dest( sassDest ) );
}

@faroukjones
Copy link

Currently, this is a feature of gulp 4 due to a limitation of node's fs.Stat object. If you need mtime to change - you need to modify the mtime on the files in your pipeline.

So it seems like will not be fixed in Gulp, we will have to find ways of handling it downstream and it sounds like they even suggest something like my gulp-touch solution. I will see if there is something I can do within themekit to handle this better without effecting other tools
Edit: It seems like even in the other issues linked in the gulp issue, other developers have solved this in a similar way.

gulp-touch (and similar gulp-touch-xxx) npm packages are 2 years out of date and don't appear to be working for gulp 4, FYI. I was able to achieve results using through2 which will accept a Writeable stream and allow me to change the atime/mtime.

const { task, watch, src, dest, series } = require('gulp');
const sass = require('gulp-sass');
const autoprefix = require('gulp-autoprefixer');
const t2 = require('through2'); // Get through2 as t2

sass.compiler = require('node-sass');

function scss(cb) {
  src('./src/scss/**/*.scss')
    .pipe(sass({outputStyle: 'expanded'}))
    .on('error', sass.logError)
    .pipe(autoprefix({
      browsers: ['> 2%', 'ie >= 10'],
      grid: true
    }))
    .pipe(t2.obj((chunk, enc, cb) => { // Execute through2
      let date = new Date();
      chunk.stat.atime = date;
      chunk.stat.mtime = date;
      cb(null, chunk);
    }))
    .pipe(dest('./assets/'))

    cb();
}

function watcher(cb) {
  watch('./src/scss/**/*.scss', series(scss))
  cb()
}

module.default = task('default', series([scss, watcher]));

This is working like a charm for me.

Brilliant! Thanks so much! Works!

@trev-dev
Copy link

Not sure if it's a gulp problem or not but since updating to ThemeKit 1.1.6 linux/amd64 I now have to use @WraithKenny's touch hack for everything that gets moved from ./src to my local Shopify theme directory. The only exception is my webpack bundled js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants