Validate @shopify/shopify_function version on build
#5153
Conversation
andrewhassan
force-pushed
the
ah.validate-shopify-function-package-version
branch
from
10e6428 to
e121a59
Compare
andrewhassan
changed the title
@shopify/shopify_function version on build
andrewhassan
force-pushed
the
ah.validate-shopify-function-package-version
branch
3 times, most recently
from
c980c84 to
e57adca
Compare
This comment has been minimized.
This comment has been minimized.
andrewhassan
force-pushed
the
ah.validate-shopify-function-package-version
branch
from
7cc70c4 to
f4c7577
Compare
Coverage report
Show files with reduced coverage 🔻
Test suite run success1995 tests passing in 902 suites. Report generated by 🧪jest coverage report action from f4c7577 |
andrewhassan
requested review from
a team,
lopert and
saga-dasgupta
and removed request for
a team
| @@ -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.
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.
I have not, but I was following the implementation here and assumed they validated this.
| }) | ||
|
|
||
| if (!packageJsonPath) { | ||
| throw new InvalidShopifyFunctionPackageError('Could not find the Shopify Functions JavaScript library.') |
There was a problem hiding this comment.
I could not get this to trigger in a way that made sense to me.
If I removed the dependency, deleted the node_modules folder, ran npm install in my extension, then tried the build command, I'd get an unrelated (?) error.
Command failed with exit code 1: npm exec -- graphql-code-generator --config package.json
But if I went in and manually removed just the node_modules/@shopify/shopify_function/package.json file, I got this message 👍
What kind of config / steps would lead to a user actually getting into this position?
There was a problem hiding this comment.
In practice, I think this could happen if the user removed the @shopify/shopify_function dependency so that it wasn't present at all in their node_modules folder.
WHY are these changes introduced?
Closes https://github.com/Shopify/shopify-functions/issues/519
The
@shopify/shopify_functionNPM package for JavaScript Functions is implicitly tied to the version of Javy used by the CLI. However, we don't validate that the NPM package is compatible today, which means it's possible to use an older version of the NPM package that isn't compatible with a newer version of Javy.WHAT is this pull request doing?
To guarantee compatibility, this PR validates that the installed
@shopify/shopify_functionpackage's major version is what the CLI expects.This PR also ensures that we're using the latest patch version in our recommendations and when generating a function.
How to test your changes?
app function buildfor a JS function usingshopify_functionversion 1.x.y.app function buildfor a JS function usingshopify_functionversion 0.x.y.app devfor a JS function (both scenarios above)app function buildfor a Rust function (should not be affected by these changes)Measuring impact
How do we know this change was effective? Please choose one:
Checklist