search

Shopify / themekit

Shopify theme development command line tool.
https://shopify.dev/tools/theme-kit
MIT License
1.19k stars 375 forks source link

watch uploads +-25kb of file #103

Closed larrybotha closed 8 years ago

larrybotha commented 9 years ago

If I watch my *.css.liquid, I can reliably expect a partial upload.

Running watch on a minified and unminified file repeatedly uploads approximately 25Kb of the file.

No options are added to config.yml.

OS: Mac OSX 10.9.5 Themekit 0.3.1

cshold commented 9 years ago

Can we put a priority on this? Makes editing css of existing themes (with some very large stylesheets) impossible. @csaunders

csaunders commented 9 years ago

Could you describe in more detail what your workflow is like here? I'm playing around with some pretty large assets (three.js and bootstrap.css) and the entire files appear to be getting uploaded correctly.

Are there lots of writes happening to the same file over and over again?

csaunders commented 9 years ago

I was able to replicate something that I think is showing what is going wrong here. If there are lots of changes happening in rapid succession to very large files, they are actually getting locked up in Shopify's background queue.

We only allow a single job to run for an asset, and with the case of a large asset (I was going from 1 byte to more than 1 megabyte) it can take a while. Unfortunately, Shopify accepts the work (even if it cannot queue it) and returns a 200.

larrybotha commented 9 years ago

With our current structure, we use Gulp to build sass from outside of the Shopify theme into the assets folder.

We have themekit watch a few folders at once, but only when sass is built will it trigger an upload for the compiled css. We don't have rapid updates to the css. When building sass, there won't be multiple other uploads, so I'm experiencing this on single file updates.

I'm pretty sure when Node closes a file after writing any sort of change will only then be picked up by themekit; not while the buffer is still receiving data.

@cshold have you experienced this with anything other than CSS? I don't think I have - probably because the stylesheets tend to be the largest single assets. I would imagine this would potentially happen with any large file.

csaunders commented 9 years ago

I'm pretty sure when Node closes a file after writing any sort of change will only then be picked up by themekit; not while the buffer is still receiving data.

Are we absolutely sure this is the true? There is no guarantee that is the case and it should be easy to test. Instead of dumping built file into the assets directory, write it somewhere else then move it. That should let us definitively know if it's an issue with how files are getting written to or if there is an underlying issue in this application.

@cshold and some colleagues have a similar setup, so I'll also give this "write then move" thing a try and see what happens.

cshold commented 9 years ago

I've can recreate this issues with theme watch on .scss.liquid and .js.liquid files, no build tasks involved.

thisislawatts commented 9 years ago

I'm also experiencing this issue with a heavyweight snippets/*.liquid file. Weighing in at an understated 129kb.

OSX 10.10.5
Theme Kit v0.3.1
csaunders commented 9 years ago

Replicated the issue and a patch that I have in https://github.com/Shopify/themekit/tree/bugfix-large-files-do-not-upload appears to fix it.

If you have the go runtime installed and want to double check that this does indeed fix the issue please do. I'll be on the #shopify IRC channel on freenode if you need any help getting your environment set up.

cshold commented 9 years ago

@stevebosworth can you test out the patch and see if it fixes the issue?

stevebosworth commented 9 years ago

Tested this branch with @csaunders a few days ago and then again today. Seems like it's :100: on my end :ship:

cshold commented 8 years ago

Can we get this in a new release?

csaunders commented 8 years ago

A release is now up on S3. All you should have to do is theme update