Functions 7452: Bundling and uploading wasm file to GCS for function extension #4445

saga-dasgupta · 2024-09-12T19:06:26Z

Issue: https://github.com/Shopify/script-service/issues/7452

WHY are these changes introduced?

To have a consistent way of uploading and obtaining extension data is core using the App Module Framework.

WHAT is this pull request doing?

This PR adds the "bundling" feature to functions and sets the output path for compilation to the bundle directory to ensure the wasm file generated is zipped and uploaded to GCS like every other extension.

The deployment plan is:
This version of CLI goes out which bundles the wasm file and uploads it (existing wasm blob upload stays in place).
Then on Core's side we verify we are able to obtain the wasm content and upload it to our runtime-engine bucket. Then we come back here and delete the uploadWasmBlob path and in core we throw an error when we fail to upload. TLDR: existing upload path stays in place till we can verify new path works.

How to test your changes?

Tophat this normally from your local computer since core changes are out. Run cli for dev and deploy both for an app with functions

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
Existing analytics will cater for this addition
PR includes analytics changes to measure impact

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes

github-actions · 2024-09-12T19:06:44Z

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

Themes: @shopify/advanced-edits
UI extensions: @shopify/ui-extensions-cli
- Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
Hydrogen: @shopify/hydrogen
Other: @shopify/app-inner-loop

packages/app/src/cli/models/extensions/extension-instance.ts

saga-dasgupta · 2024-09-13T18:22:04Z

packages/app/src/cli/models/extensions/extension-instance.ts

    }

    await this.build(options)
-
+    if (this.isFunctionExtension) {
+      await copyFile(this.outputPath, joinPath(bundleDirectory, extensionId, 'index.wasm'))


Copying the wasm file generated from the function's dist directory to the bundleDirectory for it to get zipped and uploaded later.

saga-dasgupta · 2024-09-13T18:22:32Z

packages/app/src/cli/services/deploy.test.ts

@@ -243,7 +243,7 @@ describe('deploy', () => {
    expect(updateAppIdentifiers).toHaveBeenCalledOnce()
  })

-  test('uploads the extension bundle with 1 function', async () => {
+  test('uploads the extension bundle with 1 function extension', async () => {


Keeping the test names consistent.

saga-dasgupta · 2024-09-13T18:23:34Z

packages/app/src/cli/services/deploy.test.ts

@@ -290,7 +290,7 @@ describe('deploy', () => {
      ],
      developerPlatformClient,
      extensionIds: {},
-      bundlePath: undefined,
+      bundlePath: expect.stringMatching(/bundle.zip$/),


Bundle path now has a zip file for functions' assets.

github-actions · 2024-09-17T16:30:44Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	72.67% (-0.06% 🔻)	8540/11751
🟡	Branches	69.64% (-0.06% 🔻)	4196/6025
🟡	Functions	71.73% (+0.02% 🔼)	2207/3077
🟡	Lines	73.01% (-0.06% 🔻)	8084/11073

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / extension-instance.ts	84.85% (-0.55% 🔻)	74.29% (-0.95% 🔻)	90.91% (-0.4% 🔻)	86.29% (-0.43% 🔻)
🔴	`...` / extension.ts	54.55% (-0.72% 🔻)	50%	57.14%	55.81% (-0.94% 🔻)
🟢	`...` / upload.ts	80.87% (-3.91% 🔻)	75.41% (-4.59% 🔻)	84.21% (-1.16% 🔻)	81.37% (-4.11% 🔻)
🟢	`...` / ConcurrentOutput.tsx	98.39% (-1.61% 🔻)	90.91% (-4.55% 🔻)	100%	98.33% (-1.67% 🔻)

Test suite run success

1943 tests passing in 876 suites.

Report generated by 🧪jest coverage report action from 6834807

github-actions · 2024-09-17T16:32:50Z

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

andrewhassan

This is looking great! My comments are mostly around Isaac's concerns 😄

packages/app/src/cli/models/extensions/extension-instance.ts

saga-dasgupta · 2024-09-20T20:38:11Z

packages/app/src/cli/services/build/extension.ts

    if (extension.isJavaScript) {
      await runCommandOrBuildJSFunction(extension, options)
    } else {
      await buildOtherFunction(extension, options)
    }
+    if (fileExistsSync(extension.outputPath)) {


Had to add this if condition because I see there are tests like succeeds when is a JS function and build command is not present so I assume thats the intention.

saga-dasgupta · 2024-09-20T20:42:17Z

packages/app/src/cli/services/build/extension.ts

@@ -141,11 +142,18 @@ export async function buildFunctionExtension(
  }

  try {
+    const bundlePath = joinPath(extension.outputPath.split('/').slice(0, -2).join('/'), 'index.wasm')
+    extension.outputPath = joinPath(extension.directory, joinPath('dist', 'function.wasm'))


This little dance avoids isFunctionExtension? in build.

Do you mind explaining why this step is necessary? I'm not totally following 🤔

Yes, outputPath ends up becoming <bundlepath>/dist/something.js and since <bundlepath>/dist/ does not exists it complains No such file or directory (remember that bundlePath is a /tmp directory). So, I essentially set it to the valid path that exists which is <extension directory>/dist.

packages/app/src/cli/services/dev/update-extension.ts

packages/app/src/cli/services/deploy/bundle.test.ts

packages/app/src/cli/services/build/extension.ts

jacobsteves · 2024-09-25T14:50:08Z

packages/app/src/cli/services/build/extension.ts

@@ -141,11 +142,18 @@ export async function buildFunctionExtension(
  }

  try {
+    const bundlePath = joinPath(extension.outputPath.split('/').slice(0, -2).join('/'), 'index.wasm')
+    extension.outputPath = joinPath(extension.directory, joinPath('dist', 'function.wasm'))


Do you mind explaining why this step is necessary? I'm not totally following 🤔

isaacroldan · 2024-10-01T09:50:07Z

packages/app/src/cli/models/extensions/extension-instance.ts

  async deployConfig({
    apiKey,
-    developerPlatformClient,
    appConfiguration,
  }: ExtensionDeployConfigOptions): Promise<{[key: string]: unknown} | undefined> {
-    if (this.isFunctionExtension) return this.functionDeployConfig({apiKey, developerPlatformClient, appConfiguration})
    return this.commonDeployConfig(apiKey, appConfiguration)


This is looking good!!!

I'll ask for one more change, since now deployConfig is basically doing nothing and just calling commonDeployConfig can we unify both?

Have a single deployConfig that does whatever commonDeployConfig does.
And update all calls to commonDeployConfig to call deployConfig instead. Should be straightforward :)

Indeed, thanks for catching that, it was easy to fix!

andrewhassan

I didn't 🎩 but overall the code LGTM! Nice work!

andrewhassan · 2024-10-04T18:41:08Z

packages/app/src/cli/services/build/extension.ts

+      const base64Contents = await readFile(extension.outputPath, {encoding: 'base64'})
+      await touchFile(bundlePath)
+      await writeFile(bundlePath, base64Contents)


Could we use copyFile here instead of reading and writing?

I did have copyFile here before but I need it to be in base64 format inside the bundlePath because code in core expects this to be base64 formatted, so copyFIle which copies file as is doesn't work here.

packages/app/src/cli/services/dev/update-extension.ts

isaacroldan

Awesome work @saga-dasgupta, thank you so much for all the extra clean up 🙇

packages/app/src/cli/models/app/app.ts

…ate access

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch 3 times, most recently from e0c4923 to 8c1179a Compare September 13, 2024 16:26

saga-dasgupta commented Sep 13, 2024

View reviewed changes

packages/app/src/cli/models/extensions/extension-instance.ts Outdated Show resolved Hide resolved

saga-dasgupta commented Sep 13, 2024

View reviewed changes

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch 4 times, most recently from 04ce5e5 to c5a48a8 Compare September 17, 2024 16:26

saga-dasgupta marked this pull request as ready for review September 17, 2024 16:32

andrewhassan reviewed Sep 17, 2024

View reviewed changes

packages/app/src/cli/models/extensions/extension-instance.ts Outdated Show resolved Hide resolved

packages/app/src/cli/models/extensions/extension-instance.ts Outdated Show resolved Hide resolved

saga-dasgupta requested a review from jacobsteves September 18, 2024 16:00

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch 2 times, most recently from a53dfe6 to 16a1fca Compare September 20, 2024 20:27

saga-dasgupta commented Sep 20, 2024

View reviewed changes

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch from 16a1fca to c1c12ea Compare September 20, 2024 20:40

saga-dasgupta commented Sep 20, 2024

View reviewed changes

saga-dasgupta requested review from isaacroldan and andrewhassan September 20, 2024 20:43

isaacroldan reviewed Sep 23, 2024

View reviewed changes

packages/app/src/cli/services/dev/update-extension.ts Show resolved Hide resolved

isaacroldan reviewed Sep 23, 2024

View reviewed changes

packages/app/src/cli/services/deploy/bundle.test.ts Outdated Show resolved Hide resolved

isaacroldan reviewed Sep 23, 2024

View reviewed changes

packages/app/src/cli/services/build/extension.ts Show resolved Hide resolved

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch 2 times, most recently from 4c9da9a to fc66510 Compare September 25, 2024 14:41

jacobsteves reviewed Sep 25, 2024

View reviewed changes

isaacroldan reviewed Oct 1, 2024

View reviewed changes

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch from 386b12b to 4c6bf01 Compare October 1, 2024 15:42

saga-dasgupta requested a review from isaacroldan October 1, 2024 17:49

jacobsteves approved these changes Oct 2, 2024

View reviewed changes

andrewhassan approved these changes Oct 4, 2024

View reviewed changes

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch from 4c6bf01 to 43b671e Compare October 7, 2024 14:10

isaacroldan reviewed Oct 7, 2024

View reviewed changes

packages/app/src/cli/services/dev/update-extension.ts Outdated Show resolved Hide resolved

isaacroldan approved these changes Oct 7, 2024

View reviewed changes

isaacroldan reviewed Oct 7, 2024

View reviewed changes

packages/app/src/cli/models/app/app.ts Outdated Show resolved Hide resolved

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch from 47477ba to bb7b4f0 Compare October 8, 2024 16:05

saga-dasgupta added 7 commits October 8, 2024 12:21

Bundling and uploading for functions

4c5ac0e

draftable dev works now and addressing pr comments

d15c13d

updating filepaths

95ee940

removing function wasm upload logic because core doesnt have file upd…

3913553

…ate access

addressing review comments

2862935

more clean up

650ec4c

minor changes

6834807

saga-dasgupta force-pushed the functions_7452.gcs_upload_solution branch from bb7b4f0 to 6834807 Compare October 8, 2024 17:21

saga-dasgupta added this pull request to the merge queue Oct 8, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 8, 2024

saga-dasgupta added this pull request to the merge queue Oct 8, 2024

saga-dasgupta removed this pull request from the merge queue due to a manual request Oct 8, 2024

saga-dasgupta added this pull request to the merge queue Oct 8, 2024

Merged via the queue into main with commit 58dfe9c Oct 8, 2024
36 checks passed

saga-dasgupta deleted the functions_7452.gcs_upload_solution branch October 8, 2024 18:29

shopify-shipit bot had a problem deploying to nightly October 9, 2024 03:29 Failure

andrewhassan mentioned this pull request Oct 9, 2024

Use TOML configuration for function build path #4622

Merged

5 tasks

shopify-shipit bot temporarily deployed to nightly October 10, 2024 03:32 Inactive

shopify-shipit bot temporarily deployed to production October 21, 2024 16:01 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Functions 7452: Bundling and uploading wasm file to GCS for function extension #4445

Functions 7452: Bundling and uploading wasm file to GCS for function extension #4445

saga-dasgupta commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024

saga-dasgupta Sep 13, 2024

saga-dasgupta Sep 13, 2024

saga-dasgupta Sep 13, 2024

github-actions bot commented Sep 17, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024

andrewhassan left a comment

saga-dasgupta Sep 20, 2024 •

edited

Loading

saga-dasgupta Sep 20, 2024

jacobsteves Sep 25, 2024

saga-dasgupta Sep 25, 2024 •

edited

Loading

jacobsteves Sep 25, 2024

isaacroldan Oct 1, 2024

saga-dasgupta Oct 1, 2024

andrewhassan left a comment

andrewhassan Oct 4, 2024

saga-dasgupta Oct 7, 2024 •

edited

Loading

isaacroldan left a comment

Functions 7452: Bundling and uploading wasm file to GCS for function extension #4445

Functions 7452: Bundling and uploading wasm file to GCS for function extension #4445

Conversation

saga-dasgupta commented Sep 12, 2024 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Measuring impact

Checklist

github-actions bot commented Sep 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 17, 2024 • edited Loading

Coverage report

Test suite run success

github-actions bot commented Sep 17, 2024

andrewhassan left a comment

Choose a reason for hiding this comment

saga-dasgupta Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saga-dasgupta Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewhassan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saga-dasgupta Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

isaacroldan left a comment

Choose a reason for hiding this comment

saga-dasgupta commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024 •

edited

Loading

saga-dasgupta Sep 20, 2024 •

edited

Loading

saga-dasgupta Sep 25, 2024 •

edited

Loading

saga-dasgupta Oct 7, 2024 •

edited

Loading