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

[Bug]: New notification system breaks json commands output #4925

Closed
2 tasks done
charlienaud opened this issue Nov 26, 2024 · 2 comments · May be fixed by #4930
Closed
2 tasks done

[Bug]: New notification system breaks json commands output #4925

charlienaud opened this issue Nov 26, 2024 · 2 comments · May be fixed by #4930
Labels
Type: Bug Something isn't working

Comments

@charlienaud
Copy link

Please confirm that you have:

  • Searched existing issues to see if your issue is a duplicate. (If you’ve found a duplicate issue, feel free to add additional information in a comment on it.)
  • Reproduced the issue in the latest CLI version.

In which of these areas are you experiencing a problem?

Theme

Expected behavior

When I run a command specifying json as the output format.

E.g.
shopify theme check --output json

I expect json output only.

Actual behavior

Since version 3.70.0 and the new notification system. The CLI first displays notifications then the command output.

E.g. Release notes

> shopify theme check --output json
╭─ info ─────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                                    │
│  Release notes for 3.70.0                                                                          │
│                                                                                                    │
│  Release highlights:                                                                               │
│                                                                                                    │
│    - Ruby is no longer required                                                                    │
│    - The `theme dev` command is more stable                                                        │
│    - A new notification system (expect more of these!)                                             │
│    - Numerous bug fixes                                                                            │
│                                                                                                    │
│                                                                                                    │
│                                                                                                    │
│  Read the complete release notes [1]                                                               │
│                                                                                                    │
╰────────────────────────────────────────────────────────────────────────────────────────────────────╯

But then the output is no longer a valid json, breaking systems that rely on output parsing.


There is a not yet released workaround in #4910 using env var to disable notifications.

Will this be the recommended way to opt-out from notifications in the upcoming release?
Or do you plan to automatically opt-out from notifications system, commands that return non text output?

Verbose output

+ ./node_modules/.bin/shopify --version
@shopify/cli/3.70.0 linux-arm64 node-v22.11.0
+ ./node_modules/.bin/shopify theme check --output json --verbose
2024-11-26T11:58:54.820Z: Checking if there's a version of @shopify/cli newer than 3.70.0
2024-11-26T11:58:54.820Z: Getting the latest version of NPM package: @shopify/cli
2024-11-26T11:58:54.833Z: Running command theme check
2024-11-26T11:58:54.837Z: No cached notifications found. Fetching them...
2024-11-26T11:58:54.905Z: Request to https://cdn.shopify.com/static/cli/notifications.json completed in 68 ms
With response headers:
 - cache-control: public, max-age=86400
 - content-type: application/json
 - server-timing: imagery;dur=67.983, imageryFetch;dur=66.592, cfRequestDuration;dur=19.999981
 - x-request-id: f15ee304-4cce-4972-956c-d2a281d05d27-1732547174
    
2024-11-26T11:58:54.913Z: Notifications to show: 1
╭─ info ─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                                                                                                                                        │
│  Release notes for 3.70.0                                                                                                                                                                              │
│                                                                                                                                                                                                        │
│  Release highlights:                                                                                                                                                                                   │
│                                                                                                                                                                                                        │
│    - Ruby is no longer required                                                                                                                                                                        │
│    - The `theme dev` command is more stable                                                                                                                                                            │
│    - A new notification system (expect more of these!)                                                                                                                                                 │
│    - Numerous bug fixes                                                                                                                                                                                │
│                                                                                                                                                                                                        │
│                                                                                                                                                                                                        │
│                                                                                                                                                                                                        │
│  Read the complete release notes [1]                                                                                                                                                                   │
│                                                                                                                                                                                                        │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
[1] https://github.com/Shopify/cli/releases/tag/3.70.0

[{"path":"/app/sections/header.liquid","offenses":[{"check":"LiquidHTMLSyntaxError","severity":"error","start_row":108,"start_column":1,"end_row":108,"end_column":2,"message":"SyntaxError: expected a letter, \"{{\", \"wbr\" (case-insensitive), \"track\" (case-insensitive), \"source\" (case-insensitive), \"param\" (case-insensitive), \"meta\" (case-insensitive), \"link\" (case-insensitive), \"keygen\" (case-insensitive), \"input\" (case-insensitive), \"img\" (case-insensitive), \"hr\" (case-insensitive), \"embed\" (case-insensitive), \"command\" (case-insensitive), \"col\" (case-insensitive), \"br\" (case-insensitive), \"base\" (case-insensitive), \"area\" (case-insensitive), \"svg\", \"style\", or \"script\""}],"errorCount":1,"warningCount":0,"infoCount":0},{"path":"/app/sections/main-password-footer.liquid","offenses":[{"check":"VariableName","severity":"warning","start_row":0,"start_column":10,"end_row":0,"end_column":51,"message":"The variable 'scheme1' uses wrong naming format"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/snippets/icon-with-text.liquid","offenses":[{"check":"VariableName","severity":"warning","start_row":13,"start_column":9,"end_row":13,"end_column":32,"message":"The variable 'heading_1_empty' uses wrong naming format"},{"check":"VariableName","severity":"warning","start_row":14,"start_column":9,"end_row":14,"end_column":32,"message":"The variable 'heading_2_empty' uses wrong naming format"},{"check":"VariableName","severity":"warning","start_row":15,"start_column":9,"end_row":15,"end_column":32,"message":"The variable 'heading_3_empty' uses wrong naming format"},{"check":"VariableName","severity":"warning","start_row":19,"start_column":11,"end_row":19,"end_column":33,"message":"The variable 'heading_1_empty' uses wrong naming format"},{"check":"VariableName","severity":"warning","start_row":23,"start_column":11,"end_row":23,"end_column":33,"message":"The variable 'heading_2_empty' uses wrong naming format"},{"check":"VariableName","severity":"warning","start_row":27,"start_column":11,"end_row":27,"end_column":33,"message":"The variable 'heading_3_empty' uses wrong naming format"}],"errorCount":0,"warningCount":6,"infoCount":0},{"path":"/app/sections/main-list-collections.liquid","offenses":[{"check":"VariableName","severity":"warning","start_row":19,"start_column":11,"end_row":19,"end_column":71,"message":"The variable 'moduloResult' uses wrong naming format"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/sections/main-password-header.liquid","offenses":[{"check":"VariableName","severity":"warning","start_row":101,"start_column":10,"end_row":101,"end_column":51,"message":"The variable 'scheme1' uses wrong naming format"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/sections/main-article.liquid","offenses":[{"check":"VariableName","severity":"warning","start_row":101,"start_column":21,"end_row":101,"end_column":66,"message":"The variable 'anchorId' uses wrong naming format"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/snippets/product-variant-options.liquid","offenses":[{"check":"UnusedAssign","severity":"warning","start_row":61,"start_column":4,"end_row":64,"end_column":22,"message":"The variable 'help_text' is assigned but not used"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/sections/main-search.liquid","offenses":[{"check":"UnusedAssign","severity":"warning","start_row":273,"start_column":24,"end_row":273,"end_column":152,"message":"The variable 'product_settings' is assigned but not used"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/sections/featured-product.liquid","offenses":[{"check":"UnusedAssign","severity":"warning","start_row":475,"start_column":6,"end_row":475,"end_column":47,"message":"The variable 'seo_media' is assigned but not used"}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/sections/main-product.liquid","offenses":[{"check":"UnusedAssign","severity":"warning","start_row":716,"start_column":8,"end_row":716,"end_column":49,"message":"The variable 'seo_media' is assigned but not used"},{"check":"UndefinedObject","severity":"warning","start_row":572,"start_column":44,"end_row":572,"end_column":52,"message":"Unknown object 'continue' used."}],"errorCount":0,"warningCount":2,"infoCount":0},{"path":"/app/sections/announcement-bar.liquid","offenses":[{"check":"UndefinedObject","severity":"warning","start_row":33,"start_column":11,"end_row":33,"end_column":35,"message":"Unknown object 'block' used."},{"check":"UndefinedObject","severity":"warning","start_row":22,"start_column":5,"end_row":22,"end_column":29,"message":"Unknown object 'block' used."}],"errorCount":0,"warningCount":2,"infoCount":0},{"path":"/app/layout/password.liquid","offenses":[{"check":"UndefinedObject","severity":"warning","start_row":39,"start_column":35,"end_row":39,"end_column":49,"message":"Unknown object 'scheme_classes' used."}],"errorCount":0,"warningCount":1,"infoCount":0},{"path":"/app/layout/theme.liquid","offenses":[{"check":"UndefinedObject","severity":"warning","start_row":57,"start_column":35,"end_row":57,"end_column":49,"message":"Unknown object 'scheme_classes' used."}],"errorCount":0,"warningCount":1,"infoCount":0}]

Reproduction steps

Execute shopify theme check --output json on a Dawn theme on a freshly installed 3.70.0 Shopify CLI.

TS=$(date +%Y%m%d%H%M%S) && \
git clone [email protected]:Shopify/dawn.git /tmp/dawn-$TS && \
docker run --rm -it -v /tmp/dawn-$TS:/app -w /app node:22.11.0 bash -c "set -x && npm install @shopify/[email protected] && ./node_modules/.bin/shopify --version && ./node_modules/.bin/shopify theme check --output json"

Operating System

Mac OS 14.5

Shopify CLI version (check your project's package.json if you're not sure)

3.70.0

Shell

zsh

Node version (run node -v if you're not sure)

22.11.0

What language and version are you using in your application?

No response

@tylerhellner
Copy link

Pretty sure this is fixed here: #4910

It just hasn't been released yet.

@gonzaloriestra
Copy link
Contributor

The notification has been removed, so it should work again with 3.70.0. Sorry for the inconvenience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants