Skip to content

Conversation

@saga-dasgupta
Copy link
Contributor

@saga-dasgupta saga-dasgupta commented Oct 23, 2025

Fixes https://github.com/shop/issues-shopifyvm/issues/697
Created a new CLI command shopify function app info here.
This PR runs this new command to obtain all the required and necessary info to invoke the function-runner directly in runFunction.

@saga-dasgupta saga-dasgupta force-pushed the sd.run-function-investigation branch from 614cc17 to 10ab8bf Compare October 23, 2025 19:54
@saga-dasgupta saga-dasgupta force-pushed the sd.run-function-investigation branch from 10ab8bf to fe12442 Compare October 23, 2025 20:34
@@ -0,0 +1,24 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just renamed this file, I dont know why it doesn't show as a rename.

@saga-dasgupta saga-dasgupta marked this pull request as ready for review October 23, 2025 20:41
@saga-dasgupta saga-dasgupta force-pushed the sd.run-function-investigation branch from 857c853 to 711d863 Compare October 24, 2025 16:06
@@ -1,8 +1,7 @@
{
"shopId": 55479926983,
"apiClientId": 282965573633,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned up this test fixture and moved the target up for better visibility now that we have 2 discount fixtures corresponding to different discount targets.

Comment on lines +20 to +47
let functionInfoJson;
try {
functionInfoJson = execSync(
`shopify app function info --json --path ${functionDir}`,
{
encoding: 'utf-8'
}
);
} catch (error) {
// Check if the error is due to the command not being found
if (error.message.includes('Command app function info not found')) {
throw new Error(
'The "shopify app function info" command is not available in your CLI version.\n' +
'Please upgrade to the latest version:\n' +
' npm install -g @shopify/cli@latest\n\n'
);
}
throw error;
}

const functionInfo = JSON.parse(functionInfoJson);
schemaPath = functionInfo.schemaPath;
functionRunnerPath = functionInfo.functionRunnerPath;
wasmPath = functionInfo.wasmPath;
targeting = functionInfo.targeting;

schema = await loadSchema(schemaPath);
inputQueryAST = await loadInputQuery(inputQueryPath);
}, 20000); // 20 second timeout for building the function
}, 20000); // 20 second timeout for building and obtaining information about the function
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we put this into a helper method? loadFunctionInfo / getFunctionDetails or something?
That way the test wouldn't need to handle the try/error or perform the execSync directly.

Somewhat related; I like keeping getInfo and runFunction separate, since it gives the user of an older version of the CLI the flexibiltyy of just providing the details manually if they want. (probably not a use case we actually want to support, but it's nice to have imo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes can make it a helper.
I thought in the meeting we decided we are mandating a CLI upgrade and this would be the only way of doing things.

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.

2 participants