Add support for JS code generation for flix #1222

bthaile · 2023-09-29T16:12:16Z

closes: #1232

Depends on flixkit PR onflow/flixkit-go#12

js templates will be on going iterative process in flixkit-go repo.

Description

to follow the same pattern as transactions and account, using ...

command help when running ./flow flix

execute , bindings

Usage:
  flow flix [command]

Available Commands:
  bindings    generate binding file for FLIX template fcl-js is default
  execute     execute FLIX template with a given id, name, or local filename

// Note for the future
There will be a new verb generate that will generate FLIX, so I'm reserving that word.

To Test

executecommand

flow flix execute transfer-flow 1 0x123456789
bindings command
flow flix bindings multiply.template.json

download this file to test local js generation,
https://github.com/onflow/flow-interaction-template-service/blob/master/templates/test/multiply.template.json

flow flix bindings multiply.template.json

/**
    This binding file was auto generated based on FLIX template. 
    Changes to this file might get overwritten 
**/

import * as fcl from "@onflow/fcl"
const flixTemplate = "https://flix.flow.com/v1/templates?name=transfer-flow"

const parameterNames = ["amount", "to"];
/**
* Transfer tokens from one account to another
* @param {Object} Parameters - parameters for the cadence
* @param { string } Parameters.amount - The amount of FLOW tokens to send: UFix64
* @param { string } Parameters.to - The Flow account the tokens will go to: Address
* @returns {Promise<Object>} - returns a promise which resolves to the transaction id
*/
export async function transferTokens({amount, to}) {
  const transactionId = await fcl.mutate({
    template: flixTemplate,
    args: (arg, t) => [arg(amount, t.UFix64), arg(to, t.Address)]
  });

  return transactionId
}

/**
    This binding file was auto generated based on FLIX template v1.0.0. 
    Changes to this file might get overwritten 
**/

import * as fcl from "@onflow/fcl"
import flixTemplate from "./multiply_two_integers.template.json"
 
/**
* Multiply two numbers to another
* @param {Object} Parameters - parameters for the cadence
* @param { string } Parameters.x - number to be multiplied: Int
* @param { string } Parameters.y - second number to be multiplied: Int
*/
export async function multiplyTwoIntegers({x, y}) {
  const info = await fcl.query({
    template: flixTemplate,
    args: (arg, t) => [arg(x, t.Int), arg(y, t.Int)]
  });

  return info
}

script example

For contributor use:

Targeted PR against master branch
Linked to Github issue with discussion and accepted design OR link to spec that describes this work
Code follows the standards mentioned here
Updated relevant documentation
Re-reviewed Files changed in the Github PR explorer
Added appropriate labels

chasefleming

I think the generate flag type needs to be fcl, not js since this is specifically for fcl and there could be other js libs.

Can you also link to a larger issue (probably on flixkit) which shows what the expected output is and describes the use case please?

chasefleming · 2023-10-03T21:12:48Z

internal/super/flix.go

+
+func isSaved(fr *flixResult) string {
+	if fr.save != "" {
+		return fmt.Sprintf("Generated code saved to %s", fr.save)


Use the logger instead of fmt

the isSaved method returns a string. It's a convenience method for String() and Oneliner() methods. it's not a log out statement specifically.

chasefleming · 2023-10-03T21:19:21Z

Can you also create a CLI issue as well please and link it? Thanks.

bthaile · 2023-10-16T19:09:51Z

Can you also create a CLI issue as well please and link it? Thanks.

added #1232 and referenced it in the PR description

chasefleming

@bthaile I'm getting unknown flag: --generate when running go run cmd/flow/main.go flix multiply.template.json --generate fcl-js --output multiply.template.js --network testnet.

I'm also wondering if commands like generate should be as flags --generate. Those are for things like args. generate feels like a command say flow flix generate... or flow flix execute.... Now that we are getting lots of flix items it feel like they should be subcommands otherwise you can run flow flix --bindings --execute --generate and what is it supposed to do?

internal/super/flix.go

jribbink · 2023-10-24T21:24:39Z

I'm also wondering if commands like generate should be as flags --generate. Those are for things like args. generate feels like a command say flow flix generate... or flow flix execute.... Now that we are getting lots of flix items it feel like they should be subcommands otherwise you can run flow flix --bindings --execute --generate and what is it supposed to do?

+1 on this point. In my opinion, these are distinct commands without significant functionality overlap, so they should probably be namespaced accordingly instead of distinguishing using flags.

internal/super/flix.go

bjartek · 2023-10-24T21:26:01Z

@bthaile I'm getting unknown flag: --generate when running go run cmd/flow/main.go flix multiply.template.json --generate fcl-js --output multiply.template.js --network testnet.

I'm also wondering if commands like generate should be as flags --generate. Those are for things like args. generate feels like a command say flow flix generate... or flow flix execute.... Now that we are getting lots of flix items it feel like they should be subcommands otherwise you can run flow flix --bindings --execute --generate and what is it supposed to do?

generate should be a subcommand of flix, not a flag.

internal/super/flix.go

codecov-commenter · 2023-10-25T02:04:56Z

Codecov Report

Attention: 53 lines in your changes are missing coverage. Please review.

Comparison is base (c2d0040) 38.65% compared to head (9c2ad0a) 38.07%.
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1222      +/-   ##
==========================================
- Coverage   38.65%   38.07%   -0.59%     
==========================================
  Files          38       38              
  Lines        1948     1983      +35     
==========================================
+ Hits          753      755       +2     
- Misses       1107     1140      +33     
  Partials       88       88

Flag	Coverage Δ
unittests	`38.07% <3.63%> (-0.59%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
internal/super/flix.go	`2.19% <3.63%> (+2.19%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bthaile · 2023-10-25T02:06:26Z

@bthaile I'm getting unknown flag: --generate when running go run cmd/flow/main.go flix multiply.template.json --generate fcl-js --output multiply.template.js --network testnet.

I'm also wondering if commands like generate should be as flags --generate. Those are for things like args. generate feels like a command say flow flix generate... or flow flix execute.... Now that we are getting lots of flix items it feel like they should be subcommands otherwise you can run flow flix --bindings --execute --generate and what is it supposed to do?

Updated description and PR. It makes more sense to break out verbs than add flags.

bthaile · 2023-10-25T17:00:12Z

@bjartek removed local --save flag and using global save flag. Added more structure to json output.

chasefleming · 2023-10-25T17:04:26Z

Cool @bthaile can you make an issue to remember to update the docs with this new syntax? We'll also need to add the bindings stuff to the docs.

bthaile · 2023-10-25T17:09:04Z

Cool @bthaile can you make an issue to remember to update the docs with this new syntax? We'll also need to add the bindings stuff to the docs.

good call, Added docs issue. I can write it up when this PR gets merged up and released.
onflow/docs#419

chasefleming · 2023-10-25T21:24:47Z

@bthaile There isn't much to test here but as a best practice does it make sense to at least add a test file maybe mocking the library was called so that its in place for whoever touches the code next? Apologies I should have added that when I originally put in the execute code originally. I don't think it matters that much right now since most of the logic is in the flixkit library, but if you are feeling like it wouldn't hurt.

bthaile · 2023-10-26T02:04:04Z

@bthaile There isn't much to test here but as a best practice does it make sense to at least add a test file maybe mocking the library was called so that its in place for whoever touches the code next? Apologies I should have added that when I originally put in the execute code originally. I don't think it matters that much right now since most of the logic is in the flixkit library, but if you are feeling like it wouldn't hurt.

I don't think it's needed at this time. I could add tests when adding flix generate.

chasefleming · 2023-10-26T04:15:54Z

Sounds good fine with that. Is this ready for me to manually test it again?

internal/super/flix.go

bjartek · 2023-10-27T06:01:22Z

One thing i was thinking about here is that the usecase we talked about in the developer office hours yesterday.

could a client be generated for more then 1 flix?
should a client have a name or other information. Like if we generate a go file it needs a package. If we generate jvm it needs a artifactId and a name

chasefleming · 2023-10-27T16:57:52Z

True @bjartek would be nice if it could grab a whole directory

bjartek · 2023-10-27T16:59:54Z

Not sure if it belongs in rhis issue bur a flix.json file that had lots of options could work. Let say you want to upload to a service, generate 3 different clients and maybe put those generated files somewhere. Describing all that with flags will be very complex.

bthaile · 2023-10-30T17:09:26Z

One thing i was thinking about here is that the usecase we talked about in the developer office hours yesterday.

could a client be generated for more then 1 flix?

should a client have a name or other information. Like if we generate a go file it needs a package. If we generate jvm it needs a artifactId and a name

very simple for a developer to write a script (bash, nodejs, ...) to generate package files per flix. no need to add that complexity to flow-cli.
when we get to golang generation we can figure out module name to add, don't need for js, imo. and dev can call the filename what ever. I really want to keep this simple, more features isn't better. imo

add support for js code generation for flix

f5e900f

bthaile requested review from Kay-Zee, janezpodhostnik, sideninja, DylanTinianov, chasefleming, nvdtf and jribbink as code owners September 29, 2023 16:12

bthaile marked this pull request as draft September 29, 2023 16:25

chasefleming requested changes Oct 3, 2023

View reviewed changes

verify binding value is correct, change flags name to binding

23e2c33

bthaile added 2 commits October 18, 2023 12:07

use new helper function to import fcl js code generator

f4510f8

updated flixkit-go reference to new v0.2.0

2ffb69c

bthaile marked this pull request as ready for review October 24, 2023 15:44

Merge branch 'master' into add-flix-gen

30a4e91

chasefleming changed the title ~~add support for js code generation for flix~~ Add support for JS code generation for flix Oct 24, 2023

chasefleming requested changes Oct 24, 2023

View reviewed changes

bjartek requested changes Oct 24, 2023

View reviewed changes

internal/super/flix.go Outdated Show resolved Hide resolved

jribbink reviewed Oct 24, 2023

View reviewed changes

internal/super/flix.go Outdated Show resolved Hide resolved

bjartek requested changes Oct 24, 2023

View reviewed changes

internal/super/flix.go Outdated Show resolved Hide resolved

organize flix commands, there are going to be a few so organizing now

9969b90

bthaile requested a review from bluesign as a code owner October 25, 2023 01:38

bthaile added 3 commits October 24, 2023 20:56

add transfer-tokens template example that is named on server

e153a9a

removed generate, that is a future verb

a8e3a5a

update to simplify base help message for flix

aad19cb

bthaile added 4 commits October 24, 2023 21:27

dry up code

9c2ad0a

reove local save flag and have more descriptive json output

411cc71

Merge branch 'master' into add-flix-gen

40f3c60

update go sum

91863a4

bjartek reviewed Oct 26, 2023

View reviewed changes

internal/super/flix.go Outdated Show resolved Hide resolved

bjartek approved these changes Oct 26, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

chasefleming approved these changes Oct 26, 2023

View reviewed changes

change command name from bindings to package

0009702

bthaile merged commit 670fac7 into master Oct 30, 2023
7 checks passed

bthaile deleted the add-flix-gen branch October 30, 2023 17:09

chasefleming added the Feature A new user feature or a new package API label Oct 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for JS code generation for flix #1222

Add support for JS code generation for flix #1222

bthaile commented Sep 29, 2023 •

edited

Loading

chasefleming left a comment

chasefleming Oct 3, 2023

bthaile Oct 16, 2023

chasefleming commented Oct 3, 2023 •

edited

Loading

bthaile commented Oct 16, 2023

chasefleming left a comment

jribbink commented Oct 24, 2023

bjartek commented Oct 24, 2023

codecov-commenter commented Oct 25, 2023 •

edited

Loading

bthaile commented Oct 25, 2023

bthaile commented Oct 25, 2023

chasefleming commented Oct 25, 2023

bthaile commented Oct 25, 2023

chasefleming commented Oct 25, 2023

bthaile commented Oct 26, 2023

chasefleming commented Oct 26, 2023

This comment was marked as resolved.

bjartek commented Oct 27, 2023

chasefleming commented Oct 27, 2023

bjartek commented Oct 27, 2023

bthaile commented Oct 30, 2023

Add support for JS code generation for flix #1222

Add support for JS code generation for flix #1222

Conversation

bthaile commented Sep 29, 2023 • edited Loading

Description

To Test

chasefleming left a comment

Choose a reason for hiding this comment

chasefleming Oct 3, 2023

Choose a reason for hiding this comment

bthaile Oct 16, 2023

Choose a reason for hiding this comment

chasefleming commented Oct 3, 2023 • edited Loading

bthaile commented Oct 16, 2023

chasefleming left a comment

Choose a reason for hiding this comment

jribbink commented Oct 24, 2023

bjartek commented Oct 24, 2023

codecov-commenter commented Oct 25, 2023 • edited Loading

Codecov Report

bthaile commented Oct 25, 2023

bthaile commented Oct 25, 2023

chasefleming commented Oct 25, 2023

bthaile commented Oct 25, 2023

chasefleming commented Oct 25, 2023

bthaile commented Oct 26, 2023

chasefleming commented Oct 26, 2023

This comment was marked as resolved.

bjartek commented Oct 27, 2023

chasefleming commented Oct 27, 2023

bjartek commented Oct 27, 2023

bthaile commented Oct 30, 2023

bthaile commented Sep 29, 2023 •

edited

Loading

chasefleming commented Oct 3, 2023 •

edited

Loading

codecov-commenter commented Oct 25, 2023 •

edited

Loading