-
Notifications
You must be signed in to change notification settings - Fork 218
Add stdin support for shopify app execute
#6663
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
Conversation
shopify app execute
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3434 tests passing in 1393 suites. Report generated by 🧪jest coverage report action from b4e7710 |
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
87db939 to
0a2627f
Compare
| export function isStdinPiped(): boolean { | ||
| return !process.stdin.isTTY | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that piped implies non TTY, but non TTY doesn't always mean piped 🤔
I might be wrong, but CI environments are non TTY by default and that doesn't mean you are receiving an input via stdin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! I changed it so that the function now checks what stdin actually is using fstatSync(0): isFIFO() for pipes, isFile() for file redirects.
0a2627f to
48b819c
Compare
48b819c to
d6cdf64
Compare
e9bb7f5 to
a0b57c8
Compare
d6cdf64 to
38bd5b0
Compare
38bd5b0 to
e6c20a9
Compare
a0b57c8 to
386074f
Compare
386074f to
479bccb
Compare
e6c20a9 to
bf064cc
Compare
bf064cc to
6d3b3fc
Compare
479bccb to
02b54ed
Compare
| return query | ||
| } | ||
|
|
||
| const stdinContent = await readStdin() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this is the right layer to read from STDIN? Perhaps the command layer e.g. execute where input flags etc. are being parsed would be the right moment to check whether we've been given an input text/file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I refactored it this way and made the code way more organized. Thanks!
6d3b3fc to
b4e7710
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/system.d.ts@@ -62,4 +62,23 @@ export declare function isCI(): boolean;
*
* @returns True if the current environment is a WSL environment.
*/
-export declare function isWsl(): Promise<boolean>;
\ No newline at end of file
+export declare function isWsl(): Promise<boolean>;
+/**
+ * Check if stdin has piped data available.
+ * This distinguishes between actual piped input (e.g., )
+ * and non-TTY environments without input (e.g., CI).
+ *
+ * @returns True if stdin is receiving piped data or file redirect, false otherwise.
+ */
+export declare function isStdinPiped(): boolean;
+/**
+ * Reads all data from stdin and returns it as a string.
+ * This is useful for commands that accept input via piping.
+ *
+ * @example
+ * // Usage: echo "your query" | shopify app execute
+ * const query = await readStdin()
+ *
+ * @returns A promise that resolves with the stdin content, or undefined if stdin is a TTY.
+ */
+export declare function readStdin(): Promise<string | undefined>;
\ No newline at end of file
|
dmerand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't tophat, but code LGTM. Thanks for the changes!

Resolves: https://github.com/orgs/shop/projects/208/views/34?pane=issue&itemId=139444955&issue=shop%7Cissues-api-foundations%7C1111
Inspired by: https://app.graphite.com/github/pr/Shopify/cli/6588/Initial-Setup-Bulk-Operations-Infrastructure-for-Shopify-CLI#comment-PRRC_kwDOHibhHs6WnBvj
Summary
This PR enables users to pipe GraphQL queries into
shopify app executevia stdin, improving the developer experience for bulk operations.Why This Change Matters
Before: Users had to pass queries inline via --query, which is cumbersome for complex multi-line GraphQL:
After: Users can pipe from files or other commands:
This aligns with Unix conventions and how developers already work with GraphQL tooling.
Design Decisions
cli-kit/system.tsisStdinPiped()andreadStdin()insystem.tsalongside other process utilities makes them discoverable and reusable for future commands.readStdin()returns undefined immediately when stdin is interactive. This prevents the command from hanging waiting for user input — a common pitfall in CLI tools.Thoughts for Future