themekit icon indicating copy to clipboard operation
themekit copied to clipboard

Update docs: theme new deploys current working directory & strips comments

Open th0rgall opened this issue 4 years ago • 0 comments

Describe the bug

The documentation of theme new does not fully describe what the command will do. This can result in unexpected code to be uploaded, and the stripping of config.yml comments.

Update: I noted that this is related to a very recent feature, so I guess that's why it's not documented yet. See linked issue below.

To Reproduce Steps to reproduce the behavior:

  1. run command 'theme new --env=my-environment --name=theme'
  2. run command 'theme deploy --env=my-environtment" (with a non-default theme in the local working directory)

The documentation (https://shopify.dev/tools/theme-kit/command-reference#new) specifies this:

Creates a new theme on the specified Shopify store, initializes your configuration using your API password and new theme ID, and generates and uploads default templates to make your theme valid.

But "uploads default templates" is not what is happening. The non-default current working directory theme is already being uploaded.

Heres a CLI log:

➜  moomin-shopify-theme git:(feature/germany-preview) ✗ theme new -e=test_de --name="omitted"
[omitted.myshopify.com] theme created
[omitted.myshopify.com] config created
Exists assets.
        Created assets/application.css.liquid.
        Created assets/application.js.
Exists config.
        Exists config/settings_data.json.
        Exists config/settings_schema.json.
Exists layout.
        Exists layout/theme.liquid.
Exists locales.
        Exists locales/en.default.json.
Exists templates.
        Exists templates/page.contact.liquid.
        Exists templates/page.liquid.
        Exists templates/product.liquid.
        Exists templates/article.liquid.
        Exists templates/blog.liquid.
        Exists templates/cart.liquid.
        Exists templates/index.liquid.
        Exists templates/list-collections.liquid.
        Exists templates/search.liquid.
        Exists templates/404.liquid.
        Exists templates/collection.liquid.
        Created templates/collection.list.liquid.
        Exists templates/gift_card.liquid.
Exists templates/customers.
        Exists templates/customers/register.liquid.
        Exists templates/customers/reset_password.liquid.
        Exists templates/customers/account.liquid.
        Exists templates/customers/activate_account.liquid.
        Exists templates/customers/addresses.liquid.
        Exists templates/customers/login.liquid.
        Exists templates/customers/order.liquid.
[omitted.com] uploading new files to shopify
[test_de] 318|318 [==============================================================================]  100 %
[test_de] 318 files, Updated: 318
➜  moomin-shopify-theme git:(feature/germany-preview) ✗ theme deploy --env=test_de
[test_de] 318|318 [==============================================================================]  100 %
[test_de] 318 files, No Change: 318

Note the uploading of the current working directory theme at the end of the new command

[omitted.com] uploading new files to shopify
[test_de] 318|318 [==============================================================================]  100 %
[test_de] 318 files, Updated: 318

Also, note the 318 files, No Change: 318 of the deploy command that shows that the local theme was already uploaded. I suspect this line https://github.com/Shopify/themekit/blob/b204edd3a4e1d96559bf2bc75272e0f28e4a07cc/cmd/new.go#L56 is responsible for the deploy.

A related unexpected change

The 1-sentence doc further specifies:

initializes your configuration using your API password and new theme ID

When running theme new with an --env switch, what actually happens is this:

  1. The current config.yml is read
  2. All the comments & whitespace are stripped.
  3. The newly created theme ID is filled into the env that was given.
  4. This processed config.yml is written again

I think that's good behavior, but it's not documented. I didn't know my comments were going to be stripped.

Expected behavior

  • Document that the local theme will be uploaded if it exists (I'm not sure what the behavior is if this command is run from a directory with only a config.yml present
  • Document that your current environments will not be deleted by the config initialization, but that the comments will be stripped.

Environment (please complete the following information):

  • OS [e.g. iOS]: macOS
  • Themekit version: ThemeKit 1.1.6 darwin/amd64
  • Editor [e.g. atom, sublime]: VSCode
  • Ignore: (config/settings_data.json was not ignored)
 ignore_files:
 - assets/scripts/*
 - assets/styles/

th0rgall avatar Feb 10 '21 10:02 th0rgall