Download javy and function-runner directly #4190
Conversation
|
Thanks for your contribution! Depending on what you are working on, you may want to request a review from a Shopify team:
|
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success1756 tests passing in 805 suites. Report generated by 🧪jest coverage report action from 3c963d3 |
| // The logic for determining the download URL and what to do with the response stream is _coincidentally_ the same for | ||
| // Javy and function-runner for now. Those methods may not continue to have the same logic in the future. If they | ||
| // diverge, make `Binary` an abstract class and create subclasses to handle the different logic polymorphically. | ||
| class DownloadableBinary { |
There was a problem hiding this comment.
This is an abstraction around the Javy and function-runner binaries. For the time being it's a class instead of an abstract class or interface because the way to determine the URL to use for the download and what to do with the response stream IMHO ought to be polymorphic but there is only one implementation currently. It's purely coincidental that the current implementation can be shared between them and there's no reason to expect that to continue indefinitely into the future. On the other hand, the other logic around downloading and installation should stay consistent between Javy and function-runner so that does not need to be polymorphic at this time.
|
We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset. |
|
/snapit |
|
🫰✨ Thanks @jeffcharles! Your snapshot has been published to npm. Test the snapshot by intalling your package globally: pnpm i -g @shopify/cli@0.0.0-snapshot-20240718181555
|
|
The snapshot build works! The Javy and |
| } | ||
|
|
||
| const archPlatform = `${arch}-${platform}` | ||
| // These are currently the same between both binaries _coincidentally_. |
There was a problem hiding this comment.
Is this coincidence something that might change when we migrate to a newer version?
There was a problem hiding this comment.
Yes, that would be the only time it could possibly change. I don't foresee it changing the near future.
There was a problem hiding this comment.
Got it- sounds like it's worth adding a step in the version upgrade process doc to check on this detail
There was a problem hiding this comment.
LGTM- since the versions aren't in a package manager, I think it'd be good to add some docs on how the functions team should be performing the updates. I think a short doc mentioning this file path and any other upgrade details in this repo's docs folder or our team vault docs would suffice. Given that the information isn't relevant to external contributors, the vault might be better.
I don't think we'll need more functions team eyes on this- seems fairly straightforward.
WHY are these changes introduced?
The current approach for working with Javy and function-runner is the Shopify CLI unconditionally downloads the latest released version of each tool. This means it's impossible to make breaking changes to the command line interface for these tools. Any bugs introduced in the latest released versions of these tools are also rolled out to all users of the Shopify CLI at the same time.
This change makes the Shopify CLI depend on a specific version of Javy and a specific version of function-runner. That way we can be certain that the way the Shopify CLI is invoking tools is compatible with command line interface these tools expose. This also means that if users find a regression in the version of Javy or funtion-runner that the Shopify CLI is using, they can downgrade their Shopify CLI version to use an older version of Javy or function-runner. Similarly, when a new version of Javy or function-runner is released, nightly versions of the Shopify CLI depend on these versions of Javy and function-runner and users can use a nightly version of the Shopify CLI and let us know if there are any issues that weren't caught when releasing the new versions of Javy or the function-runner.
WHAT is this pull request doing?
Instead of using
npm exec javy-cli -- [args...]ornpm exec @shopify/function-runner -- [args...]to download and install Javy and function-runner, perform the download in the Shopify CLI, and exec the downloaded binary.How to test your changes?
Assuming you are using MacOS or Linux:
pnpm shopify app build --path <path_to_app>will perform the initial download of Javy.pnpm shopify app build --path <path_to_app>will test that it uses the downloaded version of Javy.echo "{}" | pnpm shopify app function run --path <path_to_js_function_extension>will test the initial download of function-runner.echo "{}" | pnpm shopify app function run --path <path_to_js_function_extension>will it uses the downloaded version of function-runner.If you need to re-test the original downloads and are using MacOS, run
rm -r packages/app/dist/cli/services/bin.Measuring impact
How do we know this change was effective? Please choose one:
Checklist