Skip to content
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

[Functions] Bump javy to 3.1.0 #4444

Merged
merged 1 commit into from
Sep 13, 2024
Merged

[Functions] Bump javy to 3.1.0 #4444

merged 1 commit into from
Sep 13, 2024

Conversation

saulecabrera
Copy link
Member

WHY are these changes introduced?

This commit bumps javy to v3.1.0 and ammends the new commands to be used by javy to build functions.

How to test your changes?

  • Create a new JavaScript extension
  • Build it
  • Test it with function runner
  • Inspect the generated Wasm binary, it should include exports under the namespace: javy_quickjs_provider_v3

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

This commit bumps `javy` to v3.1.0 and ammends the new commands to be
used by `javy` to build functions.
Copy link
Contributor

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.

Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
72.84% (+0.11% 🔼)
8283/11371
🟡 Branches
69.45% (+0.05% 🔼)
4047/5827
🟡 Functions
71.62% (+0.03% 🔼)
2158/3013
🟡 Lines
73.19% (+0.11% 🔼)
7836/10707

Test suite run success

1863 tests passing in 847 suites.

Report generated by 🧪jest coverage report action from 3dad45a

@DuncanUszkay1
Copy link
Contributor

DuncanUszkay1 commented Sep 12, 2024

While tophatting, I ran into this issue while running the function:

error: unexpected argument '--schema-path' found

Usage: function-runner <--function <FUNCTION>|--input <INPUT>|--export <EXPORT>|--json|--profile|--profile-out <PROFILE_OUT>|--profile-frequency <PROFILE_FREQUENCY>|--codec <CODEC>>

For more information, try '--help'.
── external error ──────────────────────────────────────────────────────────────────────────────────────────────────────

Error coming from `/Users/duncanuszkay/src/github.com/Shopify/cli/packages/app/dist/cli/services/bin/function-runner -f 
/Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/dist/function.wasm --input {"cart":[]} --export run --schema-path 
/Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/schema.graphql --query-path /Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/src/run.graphql`

Command failed with exit code 2: /Users/duncanuszkay/src/github.com/Shopify/cli/packages/app/dist/cli/services/bin/function-runner -f
/Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/dist/function.wasm --input {"cart":[]} --export run --schema-path
/Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/schema.graphql --query-path /Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/src/run.graphql

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Not sure if this is just due to running from source, looking into it. Seems unlikely to be related to this PR.

function-runner version: function-runner 4.1.0

@DuncanUszkay1
Copy link
Contributor

shopify app build also returned some failures:

MacBook-Pro:cli duncanuszkay$ p shopify app build --path=a
Note that p is an alias for pnpm activated by shadowenv in the CLI repository

> @0.0.0 shopify /Users/duncanuszkay/src/github.com/Shopify/cli
> nx build cli && node packages/cli/bin/dev.js "app" "build" "--path=a"


   ✔  11/11 dependent project tasks succeeded [11 read from cache]

   Hint: you can run the command with --verbose to see the full dependent project outputs

————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————


> nx run cli:build  [existing outputs match the cache, left as is]

> pnpm tsc -b ./tsconfig.build.json


————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 NX   Successfully ran target build for project cli and 11 tasks it depends on (379ms)

Nx read the output from the cache instead of running the command for 12 out of 12 tasks.

           cv │ Building function cv...
           cv │ Building GraphQL types...
          cv2 │ Building function cv2...
          cv2 │ Building GraphQL types...
          cv2 │ (node:71288) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
          cv2 │ (Use `node --trace-deprecation ...` to show where the warning was created)
          cv2 │ Bundling JS function...
          cv2 │ Running javy...
           cv │ (node:71315) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
           cv │ (Use `node --trace-deprecation ...` to show where the warning was created)

error: unrecognized subcommand 'build'

Usage: javy <COMMAND>

For more information, try '--help'.
── external error ──────────────────────────────────────────────────────────────────────────────────────────────────────

Error coming from `/Users/duncanuszkay/src/github.com/Shopify/cli/packages/app/dist/cli/services/bin/javy build -C dynamic -C 
wit=/private/var/folders/r4/4tvrmh610xb6bcdr7lj6qz2h0000gn/T/c0600f91022dcb0041ec987aad3be112/javy-world.wit -C wit-world=shopify-function -o 
/Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/dist/function.wasm dist/function.js`

Command failed with exit code 2: /Users/duncanuszkay/src/github.com/Shopify/cli/packages/app/dist/cli/services/bin/javy build -C dynamic -C
wit=/private/var/folders/r4/4tvrmh610xb6bcdr7lj6qz2h0000gn/T/c0600f91022dcb0041ec987aad3be112/javy-world.wit -C wit-world=shopify-function -o
/Users/duncanuszkay/src/github.com/Shopify/cli/a/extensions/cv2/dist/function.wasm dist/function.js

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

error: unrecognized subcommand 'build'

Usage: javy <COMMAND>

For more information, try '--help'.
 ELIFECYCLE  Command failed with exit code 1.

@DuncanUszkay1
Copy link
Contributor

Can't replicate the issue on main, so I suppose it is an issue with this branch. Did we remove the build subcommand in javy?

Copy link
Contributor

@DuncanUszkay1 DuncanUszkay1 left a comment

Choose a reason for hiding this comment

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

Tophat unsuccessful, just marking this as reviewed so I'm properly notified when you want me to look again.

Happy to pair or slack through this as well if needed.

@saulecabrera
Copy link
Member Author

Did we remove the build subcommand in javy?

This hints to me that your javy binary is an old one. Did you try removing the javy binary in the bin directory and re-running again? FWIW, you could also check the version of the downloaded binary, it should match the version introduced in this PR (v3.1.0)

@DuncanUszkay1
Copy link
Contributor

Yep, that did it 👍 And I assume that won't be an issue for devs since it'll start out with a fresh bin on a new version

@saulecabrera
Copy link
Member Author

Yeah, that's correct.

@DuncanUszkay1
Copy link
Contributor

Getting to a build without issue, but I'm not getting the javy_quickjs_provider_v3, only javy_quickjs_provider_v2 🤔

@DuncanUszkay1
Copy link
Contributor

I noticed that the version of javy in the generated extension is only 0.1.1 in the package.json- Should something have also changed that version?

Copy link
Contributor

@DuncanUszkay1 DuncanUszkay1 left a comment

Choose a reason for hiding this comment

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

Tophatted with Saul- for a yet unknown reason, we needed to invoke the CLI using node ../packages/cli/bin/dev.js, not p shopify, that solved the issue I was having

@saulecabrera saulecabrera added this pull request to the merge queue Sep 13, 2024
Merged via the queue into main with commit 3494b53 Sep 13, 2024
37 checks passed
@saulecabrera saulecabrera deleted the bump-javy branch September 13, 2024 15:30
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