feat: support create method in AppClient #401
Conversation
Main driver behind this change is allowing the factory to use an app client to reduce duplicated code
joe-p
force-pushed
the
refactor/app_client_in_deployer
branch
from
815ffcc to
483b80f
Compare
| @@ -1222,6 +1239,9 @@ export class AppClient { | |||
| update: async (params?: AppClientBareCallParams & AppClientCompilationParams) => { | |||
| return this._algorand.createTransaction.appUpdate(await this.params.bare.update(params)) | |||
| }, | |||
| create: async (params?: AppClientBareCallParams & AppClientCompilationParams) => { | |||
| return this._algorand.createTransaction.appCreate(await this.params.bare.update(params)) | |||
There was a problem hiding this comment.
Should that be create rather than update?
| } else { | ||
| composer.addAppDelete({ appId: existingApp.appId, ...deleteParams }) |
There was a problem hiding this comment.
Are these changes (in a few places) deliberate? In general I was pretty careful about ordering of spreading but it’s possible I got some wrong
There was a problem hiding this comment.
Yes because we are now getting the params from the app client's params methods, which include the app ID of the client. Previously there was no app ID being passed in deleteParams. The order needed to be changed to ensure we use the app ID the deployer resolves, not the one in the AppClient
|
Just my 2c, If any of these would change higher level interfaces exposed to end user, would just like to remind that utils py would have to be upgraded as well. |
| @@ -260,9 +261,9 @@ export interface FundAppAccountParams { | |||
| /** Source maps for an Algorand app */ | |||
| export interface AppSourceMaps { | |||
| /** The source map of the approval program */ | |||
| approvalSourceMap: SourceMapExport | |||
| approvalSourceMap: algosdk.ProgramSourceMap | |||
There was a problem hiding this comment.
This would be extending reliance on algosdk types externally which I'm not sure we want to do.
There was a problem hiding this comment.
Ah good catch. Didn't realize the SDK used a class
Main driver behind this change is allowing the factory to use an app client to reduce duplicated code
I've now realized I've deleted some methods that are public, so we might have to add those back in to prevent MAJOR version bump. Will keep in draft until that is done