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 365 forks source link

files processed with get-chunk-name truncation fail to upload #901

Open mtshane opened 5 years ago

mtshane commented 5 years ago

Problem

@shopify/slate-tools/tools/webpack/get-chunk-name.js handles filenames that are too long and truncates them with a tilde ~, but the upload fails to push the file stating that it contains invalid characters

Replication steps

yarn deploy a project that creates a long vendor@ chunk and truncates the filename during build.

More Information

Changing to the following (hyphen) resolved the issue for us. We also reduced the name slice to 220 chars, as we still hit an ENAMETOOLONG error even with the original length.

if (name.length > 256) { name = ${name.slice(0, 220)}-${hashFilename(name)}; }

t-kelly commented 5 years ago

I anticipated this... was hoping developers wouldn't encounter it. Thanks for resurfacing it!

ghost commented 5 years ago

Is there any danger of using the above implementation to make the name shorter?

t-kelly commented 5 years ago

@george-dotdev right now, I'm using the file name to determine what pages the chunk/bundle should be loaded on with script-tags.html and style-tags.html. If we change the naming convention, we'll need to change these files as well.

ghost commented 5 years ago

The names in my theme look like this:

  template.register (783 KiB)
      vendors@layout.theme.js
      template.404@template.addresses@template.article@template.cart@template.collection@template.collection.byo-d-d@template.collection.byo-d-s@template.collection.byo-d-sd@template.collection.byo-s-d@template.collection.byo--16cc97d8.js
      template.register.js

Why are all the other templates listed underneath the template register? Is that the expected behaviour?

dotdev-brendon commented 5 years ago

@t-kelly thoughts on moving these into something like theme.globals.* and include on all pages? It's a quick solution that would simplify bundles and fix this issue...

t-kelly commented 5 years ago

@b0123498765 theme.globals.* is the equivalent of layout.theme.js (assuming you're using the layouts/theme.liquid for your theme.

dotdev-brendon commented 5 years ago

@t-kelly what about the other bundles as some of these seem to include all templates?

E.g. template.404@template.addresses@template.article@template.cart@template.collection@template.collection.byo-d-d@template.collection.byo-d-s@template.collection.byo-d-sd@template.collection.byo-s-d@template.collection.byo--16cc97d8.js

t-kelly commented 5 years ago

@b0123498765 that is the result of importing a dependency in each of the template entry point files in src/scripts/templates/*. e.g. adding import $ from 'jquery' in each template entry will result in jquery being thrown into a shared chunk named like what you mention above.

Just to confirm, I totally agree the naming convention could be reworked to be shorter -- could be done with a simple mapping object..

JPrisk commented 5 years ago

@t-kelly i've encountered this bug too, will have to rethink my code execution to avoid creating too many entry points.

mtshane commented 5 years ago

@t-kelly Any chance a fix for this might be in the works, or even some direction on how to resolve it locally for a project? It seems like with the truncation, some of our pages work, and some don't.. I'm assuming that js file just isn't being loaded on the pages where their template name is truncated off the filename?

agustinbranaf commented 4 years ago

@mtshane @JPrisk @b0123498765 Did you manage to implement any fix for this?