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

refactor: prereqs for puya debugging support #314

Closed
wants to merge 17 commits into from

Conversation

aorumbayev
Copy link
Contributor

@aorumbayev aorumbayev commented Sep 6, 2024

Proposed Changes

  • Deprecating persistSourceMaps
  • Additionally addresses issues with node specific debug utils, making utils-ts isomorphic and splitting node specific debug utils into a separate package algokit-utils-debug

Further Notes

Just a draft - can be used with per-branch installed as part of related work streams on puya debugging support

@aorumbayev
Copy link
Contributor Author

@neilcampbell the test in the debug-utils is more of an integration test, but tested it out by packing local modules and trying it with ts deployer from puya template. The only extra requirements with this is user needs to import that extra package (which triggers the function that registers the handlers with config).

@neilcampbell neilcampbell changed the base branch from main to app-client-deprecation September 17, 2024 13:36
@@ -0,0 +1,68 @@
# AlgoKit TypeScript Utilities
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably want a tailored readme for this package. Also we should be able to remove the NextJS compatibility section in the main readme.

"name": "@algorandfoundation/algokit-utils-debug",
"version": "0.1.0",
"private": false,
"description": "A set of core Algorand utilities written in TypeScript and released via npm that make it easier to build solutions on Algorand.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably want a more tailored description.

@@ -0,0 +1,163 @@
{
"name": "@algorandfoundation/algokit-utils-debug",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we planning on versioning to match utils-ts? Feels a bit weird to me.
I'm wondering if this alone warrants splitting into a seperate repo.

@@ -27,7 +27,6 @@ Config.configure({

Debugging utilities can be used to simplify gathering artifacts to be used with [AlgoKit AVM Debugger](https://github.com/algorandfoundation/algokit-avm-vscode-debugger) in non algokit compliant projects. The following methods are provided:

- `persistSourceMaps`: This method persists the sourcemaps for the given sources as AlgoKit AVM Debugger compliant artifacts. It accepts an array of `PersistSourceMapInput` objects. Each object can either contain `rawTeal`, in which case the function will execute a compile to obtain byte code, or it can accept an object of type `CompiledTeal` provided by algokit, which is used for source codes that have already been compiled and contain the traces. It also accepts the root directory of the project, an `Algodv2` client to perform the compilation, and a boolean indicating whether to include the source files in the output.
- `simulateAndPersistResponse`: This method simulates the atomic transactions using the provided `AtomicTransactionComposer` object and `Algodv2` object, and persists the simulation response to an AlgoKit AVM Debugger compliant JSON file. It accepts the `AtomicTransactionComposer` with transaction(s) loaded, an `Algodv2` client to perform the simulation, the root directory of the project, and the buffer size in megabytes.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no longer exported as well, so should be remove from the docs. This would be a breaking change as well.
Are we happy that it isn't being used?

@@ -27,7 +27,6 @@ Config.configure({

Debugging utilities can be used to simplify gathering artifacts to be used with [AlgoKit AVM Debugger](https://github.com/algorandfoundation/algokit-avm-vscode-debugger) in non algokit compliant projects. The following methods are provided:
Copy link
Contributor

Choose a reason for hiding this comment

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

Docs will need to be updated to include the new way of activating the debug utils.

import { persistSourceMaps, simulateAndPersistResponse } from './debugging'

// Automatically register debug handlers upon import
Config.registerDebugHandler(async (params: DebugParams) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method of activation feels a bit strange to me. I personally feel it's better to export a function which can be called to register the handlers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@neilcampbell how about Config.configure method taking an extra "addons" key where value is list of callbacks to invoke? The main thing i wanted to avoid is to force user doing extra actions on top of indicating via debug flag that user wants to enable this functionality but explicit addons key might separate the concerns a bit in this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

another thing to keep in mind is the fact that after this potential split of the package, there will be slight discrepancy on utils-py because we wouldn't need to do the same split there, hence why I tried to keep Config.configure interface unchanged and only require to do the import of the package in case utils-ts is used and debug utils are needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah for sure that's something to consider. Maybe we can do a quick design pairing session.

@neilcampbell neilcampbell changed the base branch from app-client-deprecation to main September 17, 2024 14:07
aorumbayev and others added 17 commits September 20, 2024 15:27
BREAKING CHANGE: `ExecuteParams` has been moved from `types/composer` to `types/transaction`
…create and deploy apps and create app clients

test: Added test coverage of AppClient and AppFactory

BREAKING CHANGE: `persistSourceMaps` takes `appManager` rather than `client` now
feat: Added appClient.params.fundAppAccount and appClient.transactions.fundAppAccount
fix: Allow extraProgramPages to be passed into create and deploy call as override
feat: Added a number of methods and types to ARC-56 to make interacting with it easier
@aorumbayev aorumbayev closed this Sep 20, 2024
@aorumbayev aorumbayev deleted the feat/puya-debugging branch September 20, 2024 18:10
@aorumbayev
Copy link
Contributor Author

@neilcampbell moved to a separate pr since i restarted from new branch to avoid dealing with merge conflicts

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.

3 participants