feat: improve UX for Typescript app codegen

[Niraj-Kamdar]

Update App Template: polywrap/wrap-cli#1855

[krisbitney]

This looks good but I think we could reduce the number of getters for each property. I'm guessing you thought a lot about this so maybe I'm missing something.

Since the getter methods are abstract, the user will always implement them. Since they will always be implemented, I think it would make more sense to just make the properties themselves abstract and public, or to remove the properties altogether and just use the methods.

For example,

// is this._defaultUri redundant?
private _getUri(uri?: string): string {
  return uri || this._defaultUri || this._getDefaultUri() || "testimport.uri.eth";
}
  // can we do this?
private _getUri(uri?: string): string {
  return uri || this._getDefaultUri() || "testimport.uri.eth";
}

Other than that question, I think this is very good. It gives the user sane defaults (the likely URI) for the common case, while allowing full customization of an abstract class.

Another suggestion I just thought of: should the class validate the URI it is constructed with? (using client.validate)

[Niraj-Kamdar]

@krisbitney
The suggestion to remove this._defaultUri from the getter method and rely solely on this._getDefaultUri() might seem reasonable at first glance. However, there is a performance consideration to take into account.

When using this._defaultUri in the getter method, we're effectively caching the default URI value. Caching means that once the default URI is computed (which usually involves some calculation or processing), it is stored in memory and can be reused without recomputation as long as it remains unchanged.

On the other hand, calling this._getDefaultUri() directly every time the getter is accessed may lead to unnecessary recomputation of the default URI. Depending on the implementation of _getDefaultUri(), this could involve executing complex logic or fetching data from external sources, which can be a performance bottleneck if called frequently.

By keeping this._defaultUri in the getter method and using the || operator, we ensure that we only compute the default URI once, the first time it is needed during construction. Subsequent calls to the getter will use the cached value, which improves performance by avoiding redundant computations.

To demonstrate this, let's consider an example scenario with multiple calls to the getter method:

// Using _defaultUri in the getter method (with caching)
private _defaultUri: string | undefined;

private _getUri(uri?: string): string {
  return uri || this._defaultUri || this._getDefaultUri() || "testimport.uri.eth";
}

// Without _defaultUri in the getter method (no caching)
private _getUri(uri?: string): string {
  return uri || this._getDefaultUri() || "testimport.uri.eth";
}

// Example usage
const instance = new MyClass(); // First call - _getDefaultUri() will be executed and result will be cached in _defaultUri

// Subsequent calls - _getDefaultUri() won't be executed again, cached value will be used
const uri1 = instance.uri; // defaultUri
const uri2 = instance._getUri(); // defaultUri 
// ...

In the first scenario (with caching), _getDefaultUri() is only executed once, and subsequent calls to instance.uri reuse the cached value. This provides better performance and reduces unnecessary computational overhead.

In contrast, in the second scenario (without caching), every call to instance.uri will trigger _getDefaultUri() again, which can lead to redundant computations, especially if the default URI calculation is resource-intensive.

In conclusion, keeping this._defaultUri in the getter method and using it in conjunction with the || operator is beneficial as it caches the default URI value, improving performance by avoiding unnecessary recomputation.

I appreciate you pointing this out though, due to this discussion, I realized that it would be better to have those protected getters as async functions and we may further lazily compute the values during first call and support more use-cases of fetching the needed values asynchronously.

[krisbitney]

@krisbitney The suggestion to remove this._defaultUri from the getter method and rely solely on this._getDefaultUri() might seem reasonable at first glance. However, there is a performance consideration to take into account.

When using this._defaultUri in the getter method, we're effectively caching the default URI value. Caching means that once the default URI is computed (which usually involves some calculation or processing), it is stored in memory and can be reused without recomputation as long as it remains unchanged.

On the other hand, calling this._getDefaultUri() directly every time the getter is accessed may lead to unnecessary recomputation of the default URI. Depending on the implementation of _getDefaultUri(), this could involve executing complex logic or fetching data from external sources, which can be a performance bottleneck if called frequently.

By keeping this._defaultUri in the getter method and using the || operator, we ensure that we only compute the default URI once, the first time it is needed during construction. Subsequent calls to the getter will use the cached value, which improves performance by avoiding redundant computations.

To demonstrate this, let's consider an example scenario with multiple calls to the getter method:

// Using _defaultUri in the getter method (with caching)
private _defaultUri: string | undefined;

private _getUri(uri?: string): string {
  return uri || this._defaultUri || this._getDefaultUri() || "testimport.uri.eth";
}

// Without _defaultUri in the getter method (no caching)
private _getUri(uri?: string): string {
  return uri || this._getDefaultUri() || "testimport.uri.eth";
}

// Example usage
const instance = new MyClass(); // First call - _getDefaultUri() will be executed and result will be cached in _defaultUri

// Subsequent calls - _getDefaultUri() won't be executed again, cached value will be used
const uri1 = instance.uri; // defaultUri
const uri2 = instance._getUri(); // defaultUri 
// ...

In the first scenario (with caching), _getDefaultUri() is only executed once, and subsequent calls to instance.uri reuse the cached value. This provides better performance and reduces unnecessary computational overhead.

In contrast, in the second scenario (without caching), every call to instance.uri will trigger _getDefaultUri() again, which can lead to redundant computations, especially if the default URI calculation is resource-intensive.

In conclusion, keeping this._defaultUri in the getter method and using it in conjunction with the || operator is beneficial as it caches the default URI value, improving performance by avoiding unnecessary recomputation.

I appreciate you pointing this out though, due to this discussion, I realized that it would be better to have those protected getters as async functions and we may further lazily compute the values during first call and support more use-cases of fetching the needed values asynchronously.

lol, thanks NirajGPT

[Niraj-Kamdar]

lol, thanks NirajGPT
You caught me! But you gotta acknowledge, it really makes it easy to understand! haha!

[krisbitney]

lol, thanks NirajGPT
You caught me! But you gotta acknowledge, it really makes it easy to understand! haha!

For sure!

[dOrgJelli]

This PR should point to a new PR in the CLI for the updated template project. This way we know what the e2e looks like.

[dOrgJelli]

This type isn't being used anywhere, is this what the user imports into their code? If so I don't think that's what we want, instead we'd want import { TestImport } from ...

[dOrgJelli]

Created a PR here @Niraj-Kamdar polywrap/wrap-cli#1855

[dOrgJelli]

🔥