Skip to content

Conversation

@nickwesselman
Copy link
Contributor

@nickwesselman nickwesselman commented Dec 18, 2025

WHY are these changes introduced?

With the addition of --query-file, we now have 3 ways of specifying the GraphQL query, which is a bit excessive. Reading queries from STDIN is really only useful for echo or cat, and --query and --query-file cover those cases respectively.

Testing of these commands in scripts makes it apparent that if anything, variables should be passable via STDIN. Let's hold off on that, but meanwhile let's reserve STDIN for that possibility.

WHAT is this pull request doing?

  • Replaces the exclusive flag constraint with exactlyOne for query input flags to ensure users provide either --query or --query-file
  • Removes stdin fallback for query input, requiring explicit query specification
  • Updates flag descriptions to remove references to stdin input
  • Updates error handling to use BugError when flag validation fails
  • Simplifies the prepareExecuteContext function by removing the command name parameter

How to test your changes?

  1. Try running shopify app execute without providing either --query or --query-file
  2. Verify that you get a clear error message requiring one of these flags
  3. Test with valid --query parameter
  4. Test with valid --query-file parameter
  5. Try using both flags together and verify it fails with appropriate error

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Contributor Author

nickwesselman commented Dec 18, 2025

@github-actions
Copy link
Contributor

github-actions bot commented Dec 18, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 79.51% 14195/17852
🟡 Branches 73.62% 6967/9464
🟡 Functions 79.63% 3640/4571
🟡 Lines 79.89% 13421/16800

Test suite run success

3567 tests passing in 1416 suites.

Report generated by 🧪jest coverage report action from 6d211c7

@nickwesselman nickwesselman marked this pull request as ready for review December 18, 2025 14:25
@nickwesselman nickwesselman requested review from a team as code owners December 18, 2025 14:25
@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@nickwesselman nickwesselman changed the title remove STDIN support on execute commands remove STDIN support on execute commands Dec 18, 2025
@nickwesselman nickwesselman force-pushed the execute_commands_remove_query_stdin_support branch from 5489167 to 6d211c7 Compare December 18, 2025 15:13
Copy link
Contributor

Just curious, what from the testing made you conclude that variables should be passable via STDIN, instead of queries?

Copy link
Contributor

@szluzero szluzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great DX improvement!

Copy link
Contributor Author

Just curious, what from the testing made you conclude that variables should be passable via STDIN, instead of queries?

Good question. The fact that I was using previous query results to drive mutations primarily. And realizing how damn powerful jq is. I could easily see scripts that do something like shopify app execute --query <query> | jq <transformation and merging> | shopify app execute --query <mutation>

Base automatically changed from app_execute_query_file to main December 19, 2025 18:18
@nickwesselman nickwesselman added this pull request to the merge queue Dec 19, 2025
Merged via the queue into main with commit 7297a1f Dec 19, 2025
25 of 45 checks passed
@nickwesselman nickwesselman deleted the execute_commands_remove_query_stdin_support branch December 19, 2025 18:26
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

Successfully merging this pull request may close these issues.

5 participants