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

Initial set up of ccd-js-gen package #242

Merged
merged 10 commits into from
Sep 6, 2023
Merged

Initial set up of ccd-js-gen package #242

merged 10 commits into from
Sep 6, 2023

Conversation

limemloh
Copy link
Collaborator

@limemloh limemloh commented Aug 25, 2023

Purpose

Introduce the ccd-js-gen package for generating typed JavaScript code.
Closes #236

Changes

  • Introduce the ccd-js-gen package capable of generating simple smart contract clients (not taking advantage of the schema yet).
  • Added a ccd-js-gen example with a script to fetch the module source for wCCD to generate a client, and a script to use this generated client.

In the SDK:

  • Added Module class for functionality related to smart contract modules, such as parsing the WebAssembly and interface of the module.
  • Added Smart contract related types ContractName, EntrypointName and helper functions isInitName, isReceiveName, getContractNameFromInit and getNamesFromReceive.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

@limemloh limemloh marked this pull request as ready for review August 25, 2023 11:01
packages/common/src/types/Module.ts Outdated Show resolved Hide resolved
packages/common/src/types/Module.ts Outdated Show resolved Hide resolved
packages/common/src/types/Module.ts Outdated Show resolved Hide resolved
packages/common/src/types/Module.ts Outdated Show resolved Hide resolved
packages/common/src/types/Module.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Show resolved Hide resolved
@limemloh limemloh requested a review from abizjak August 29, 2023 14:37
@limemloh limemloh force-pushed the setup-ccd-js-gen branch 2 times, most recently from 03764d6 to 1bc23bc Compare August 30, 2023 07:03
Copy link
Collaborator

@soerenbf soerenbf left a comment

Choose a reason for hiding this comment

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

Cool to see the output client! Here are some things to consider:

packages/ccd-js-gen/src/lib.ts Show resolved Hide resolved
packages/common/src/types/Module.ts Outdated Show resolved Hide resolved
packages/common/src/types/Module.ts Show resolved Hide resolved
examples/ccd-js-gen/generate.ts Outdated Show resolved Hide resolved
examples/ccd-js-gen/generate.ts Outdated Show resolved Hide resolved
examples/ccd-js-gen/generate.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
examples/ccd-js-gen/client-tokenMetadata.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@abizjak abizjak left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but I am wondering why the wCCD examples are not nested under a directory that reflects that.

Now there is just ccd-js-gen/generate.ts. Why is this not scoped under something like

ccd-js-gen/wCCD/generate.ts`

?

examples/ccd-js-gen/generate.ts Outdated Show resolved Hide resolved
examples/ccd-js-gen/generate.ts Outdated Show resolved Hide resolved
examples/ccd-js-gen/client-tokenMetadata.ts Outdated Show resolved Hide resolved
@limemloh limemloh changed the base branch from cis4 to main September 5, 2023 07:24
@limemloh limemloh force-pushed the setup-ccd-js-gen branch 5 times, most recently from 58c1128 to 7e8e6d7 Compare September 5, 2023 07:54
Copy link
Collaborator

@soerenbf soerenbf left a comment

Choose a reason for hiding this comment

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

I still think it would be nice to add the option for explicitly specifying which entrypoints (with receive/view being specified separately) as it's a 1-time setup thing.

We could even provide feedback in the terminal as to which entrypoints have been

  • Included in the generated client
  • Excluded from the generated client
  • Not found (maybe throw an error here?).

The advantages here being:

  • Users will not try sending view invocations as transactions by mistake (e.g. it would be reasonable to expect, as a user, that to get cis2 balance you would do client.balanceOf instead of client.dryRun.balanceOf).
  • Possibility to include only the code you need in your bundle (tree shaking does not work on classes, as you know)

Disadvantages:

  • Users will have to change generate config when contract changes, as opposed to only changing the code interfacing with the generated client
  • Users will have to know up-front which entrypoints are view/receive functions instead of through trial/error when coding

What are your thoughts on this? Of course this also depends on the expected timeline for being able to identify view/receive functions in a contract

packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
packages/ccd-js-gen/src/lib.ts Outdated Show resolved Hide resolved
@limemloh
Copy link
Collaborator Author

limemloh commented Sep 6, 2023

@soerenbf

I still think it would be nice to add the option for explicitly specifying which entrypoints (with receive/view being specified separately) as it's a 1-time setup thing.

We could even provide feedback in the terminal as to which entrypoints have been

* Included in the generated client

* Excluded from the generated client

* Not found (maybe throw an error here?).

The advantages here being:

* Users will not try sending view invocations as transactions by mistake (e.g. it would be reasonable to expect, as a user, that to get cis2 balance you would do `client.balanceOf` instead of `client.dryRun.balanceOf`).

* Possibility to include only the code you need in your bundle (tree shaking does not work on classes, as you know)

Disadvantages:

* Users will have to change generate config when contract changes, as opposed to only changing the code interfacing with the generated client

* Users will have to know up-front which entrypoints are view/receive functions instead of through trial/error when coding

What are your thoughts on this? Of course this also depends on the expected timeline for being able to identify view/receive functions in a contract

Great points you are bringing up, and I think we need to reconsider some of my initial design decisions, especially related to producing tree-shakable code. Regarding your suggestion of taking a contract description when generating the client, I would suggest keeping this for a future iteration of the tool and let it generate everything for now.

Since the primary goal of this PR is to set up the project and give a starting point, I suggest we get it merged and do the changes afterward.

packages/ccd-js-gen/src/lib.ts Show resolved Hide resolved
@limemloh limemloh merged commit 8012dec into main Sep 6, 2023
9 checks passed
@limemloh limemloh deleted the setup-ccd-js-gen branch September 6, 2023 15:44
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.

Set up smart contract generator package
3 participants