-
Notifications
You must be signed in to change notification settings - Fork 143
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
Validate @shopify/shopify_function
version on build
#5153
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'@shopify/app': patch | ||
--- | ||
|
||
Validate the @shopify/shopify_function NPM package version is compatible with the Javy version |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,14 +9,34 @@ | |
import {exec} from '@shopify/cli-kit/node/system' | ||
import {dirname, joinPath} from '@shopify/cli-kit/node/path' | ||
import {build as esBuild, BuildResult} from 'esbuild' | ||
import {findPathUp, inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' | ||
import {findPathUp, inTemporaryDirectory, readFile, writeFile} from '@shopify/cli-kit/node/fs' | ||
import {AbortSignal} from '@shopify/cli-kit/node/abort' | ||
import {renderTasks} from '@shopify/cli-kit/node/ui' | ||
import {pickBy} from '@shopify/cli-kit/common/object' | ||
import {runWithTimer} from '@shopify/cli-kit/node/metadata' | ||
import {AbortError} from '@shopify/cli-kit/node/error' | ||
import {Writable} from 'stream' | ||
|
||
export const SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '1' | ||
|
||
class InvalidShopifyFunctionPackageError extends AbortError { | ||
constructor(message: string) { | ||
super( | ||
message, | ||
outputContent`Make sure you have a compatible version of the ${outputToken.yellow( | ||
'@shopify/shopify_function', | ||
)} library installed.`, | ||
[ | ||
outputContent`Add ${outputToken.green( | ||
`"@shopify/shopify_function": "~${SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION}.0.0"`, | ||
)} to the dependencies section of the package.json file in your function's directory, if not already present.` | ||
.value, | ||
`Run your package manager's install command to update dependencies.`, | ||
], | ||
) | ||
} | ||
} | ||
|
||
interface JSFunctionBuildOptions { | ||
stdout: Writable | ||
stderr: Writable | ||
|
@@ -117,19 +137,7 @@ | |
}) | ||
|
||
if (!entryPoint || !runModule) { | ||
throw new AbortError( | ||
'Could not find the Shopify Functions JavaScript library.', | ||
outputContent`Make sure you have the latest ${outputToken.yellow( | ||
'@shopify/shopify_function', | ||
)} library installed.`, | ||
[ | ||
outputContent`Add ${outputToken.green( | ||
'"@shopify/shopify_function": "1.0.0"', | ||
)} to the dependencies section of the package.json file in your function's directory, if not already present.` | ||
.value, | ||
`Run your package manager's install command to update dependencies.`, | ||
], | ||
) | ||
throw new InvalidShopifyFunctionPackageError('Could not find the Shopify Functions JavaScript library.') | ||
} | ||
|
||
if (!fun.entrySourceFilePath) { | ||
|
@@ -139,11 +147,32 @@ | |
return entryPoint | ||
} | ||
|
||
async function validateShopifyFunctionPackageVersion(fun: ExtensionInstance<FunctionConfigType>) { | ||
const packageJsonPath = await findPathUp('node_modules/@shopify/shopify_function/package.json', { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. have you tried this path with different package managers? I know that pnpm sometimes uses a different folder structure for modules. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have not, but I was following the implementation here and assumed they validated this. |
||
type: 'file', | ||
cwd: fun.directory, | ||
}) | ||
|
||
if (!packageJsonPath) { | ||
throw new InvalidShopifyFunctionPackageError('Could not find the Shopify Functions JavaScript library.') | ||
andrewhassan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const packageJson = JSON.parse(await readFile(packageJsonPath)) | ||
const majorVersion = packageJson.version.split('.')[0] | ||
|
||
if (majorVersion !== SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION) { | ||
throw new InvalidShopifyFunctionPackageError( | ||
'The installed version of the Shopify Functions JavaScript library is not compatible with this version of Shopify CLI.', | ||
) | ||
} | ||
} | ||
|
||
export async function bundleExtension( | ||
fun: ExtensionInstance<FunctionConfigType>, | ||
options: JSFunctionBuildOptions, | ||
processEnv = process.env, | ||
) { | ||
await validateShopifyFunctionPackageVersion(fun) | ||
const entryPoint = await checkForShopifyFunctionRuntimeEntrypoint(fun) | ||
|
||
const esbuildOptions = { | ||
|
@@ -276,6 +305,7 @@ | |
} | ||
|
||
async bundle(fun: ExtensionInstance<FunctionConfigType>, options: JSFunctionBuildOptions, processEnv = process.env) { | ||
await validateShopifyFunctionPackageVersion(fun) | ||
await checkForShopifyFunctionRuntimeEntrypoint(fun) | ||
|
||
const contents = this.entrypointContents | ||
|
@@ -333,7 +363,7 @@ | |
} | ||
|
||
export function jsExports(fun: ExtensionInstance<FunctionConfigType>) { | ||
const targets = fun.configuration.targeting || [] | ||
Check warning on line 366 in packages/app/src/cli/services/function/build.ts GitHub Actions / ESLint Report Analysispackages/app/src/cli/services/function/build.ts#L366
|
||
const withoutExport = targets.filter((target) => !target.export) | ||
const withExport = targets.filter((target) => Boolean(target.export)) | ||
|
||
|
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.
Should I be able to see these messages in the console when I encounter this error?
I do see the passed in message:
Could not find the Shopify Functions JavaScript library.
I know in this PR you refactored things into an AbortError subclass so this is just a mov, but still got me thinking... where does all this outputContent end up? Even running the build command with
--verbose
doesn't show it.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 noticed that too when testing. It appears that the error is caught and rethrown here. I didn't want to tackle that as part of this PR though, since it's more related to the general error handling.
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.
Are you going to fix that in a follow-up PR? there's no reason to have a custom Error if doesn't work as expected :)
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 can take a look. I'm not actually sure why we rethrow the error like this 😅 It looks like you added it about a month ago. Do you have context on it?
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.
Ah, I added that try/catch because we were leaking some errors, and we want to properly Abort if there is an issue in any step of the build process.
But the rethrow keeps the message, why isn't it preserved in this case?
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.
Sorry I missed this comment! We do keep the message, but we don't keep the other parameters (
tryMessage
,nextSteps
) here.