demergent-labs / azle

A WebAssembly runtime for TypeScript and JavaScript on ICP
MIT License
197 stars 34 forks source link

Type Aliases won't work if they are renamed during import or if they have a qualified name #1116

Closed bdemann closed 10 months ago

bdemann commented 1 year ago

There are two kinds of Aliases in this world

  1. Those that show up in the alias table
  2. Those that show up in the CandidTypes.aliases

The ones that show up in the alias table all get resolved to the actual azle/candid type. I think these are pretty solid, and I don't think they need any additional work. So cast these from your mind. The main way these can be improved is additional testing, making the code more declarative, and making it so that we could maybe minimize some of the code duplication that happens with the other type of aliases.

The ones that show up in the CandidTypes.aliases don't get resolved at all. They get thrown in the lib.rs and if the right hand side is not defined then we get an error. The idea is it's on the developer to make sure that every alias they provide is of a valid type.

So what kind of things will show up in this second list of aliases?

It should be exclusively made up of aliases to developer or azle defined Records, Variants, Tuples, Funcs, and maybe Services.

So any aliases to Result would be in this list, because Result is an azle defined variant. If the developer made a record called User. Then User would end up in the list of Records and an aliases to User will end up in the list of type aliases.

Here is the problem that I am running into.

Because we just make the aliases and just hope that the right hand side is going to be there in the exact name and configuration we run into a lot of the same problems as when we were just hoping that people would use "Record" to decorate their records without taking into account that they might rename azle.Record to something else entirely.

If they type alias is a simple alias we have that covered.

However if the type alias is renamed via an import or a qualified name we will not have that in the list with the current system.

In other words

import {Result} from 'azle';

type DirectAliasToResult = Result<int8, string>;
type LayeredAliasToResult = DirectAliasToResult;

works fine. Both DirectAliasToResult and LayeredAliasToResult will show up in the CandidTypes.aliases list and since Result is in the CandidTypes.variants list it will all work.

On the other hand

import * as azle from 'azle';
import {Result as ImportedResult} from 'azle';

type NamespacedResult = azle.Result<nat8, string>;
type AliasToRenamedResult = ImportedResult<boolean, string>;

NamespacedResult and AliasToRenamedResult might show up in CandidTypes.aliases (I actually think there is a bug so they might not) azle.Result and ImportedResult do not, so even though Result is in the CandidTypes.variants list, these type aliases will not work because type.Result and ImportedResult need further resolution.

One possible solution is to get rid of any intermediate aliases. So in the first example the generated rust file currently would look like this:

enum _AzleResult<Ok, Err> = {Ok(Box<Ok>), Err(Box<Err>)};
type DirectAlias = _AzleResult<i8, String>;
type LayeredAliasToResult = DirectAlias;

What if we updated it so that it looked like this:

enum _AzleResult<Ok, Err> = {Ok(Box<Ok>), Err(Box<Err>)};
type DirectAlias = _AzleResult<i8, String>;
type LayeredAliasToResult = _AzleResult<i8, String>;

Then the other examples would look like this

type NamespacedResult = _AzleResult<u8, String>;
type AliasToRenamedResult = _AzleResult<bool, String>

As opposed to

type NamespacedResult = namespace::Result<u8, String>;
type ImportedResult<Ok, Err> = _AzleResult<Ok, Err>
type AliasToRenamedResult = ImportedResult<bool, String>

or however we were able to figure out how to represent it.

This issue with the generics is making me think that maybe a hybrid approach might be best. Having to figure out the concrete implementation of those seem problematic. But with the imports and the namespaces those don't deal with the generics at all, its just renaming. So we somehow track down which type it's referring to and replace it with that once we get down to the generated lib file

One of the unfortunate parts of this solution is that we would need to

  1. Be able to figure out what the type refers to
  2. Figure out the concrete implementation of the type it refers to. That is to say while we are in typescript we can figure out if it's an azle type, but figuring out if it's a record or variant etc and making sure it's formed correctly and is a valid type and not just another type alias.

The problem is that the first thing we do in typescript. And the second thing we don't have all of the information until we have run it though rust.

Long story short I don't have a good solution for this problem yet.

Here are some things that I cooked up to test it.

First, all of the above results examples I think would be pretty good.

Next, the reason I ran into this in the first place was because I was trying to test out how type aliases would work if they had to go through a * import.

I had tried to do that and it didn't quite work because the resolution went down the qualified name path instead of the export declarations path. So it's a little complicated for that reason. So maybe it can just be greatly simplified now that I understand the actual problem.

With that being said there was a bit of a problem with the qualified names if they were coming through a * export.

// TODO get this more robust test working
import { MixedStarRecord, MixedConcreteStar } from './types';
import {
    MixedStarRecord as ImportedRecord,
    MixedConcreteStar as ImportedConcrete
} from './types';
type FakeConcreteStar = MixedConcreteStar;
type FakeMixedStarRecord = MixedStarRecord;
azle.$query;
export function compareStars(
    record: MixedStarRecord,
    concrete: MixedConcreteStar,
    fakeRecord: FakeMixedStarRecord,
    fakeConcrete: FakeConcreteStar,
    typeRecord: types.MixedStarRecord,
    typeConcrete: types.MixedConcreteStar,
    importedRecord: ImportedRecord,
    importedConcrete: ImportedConcrete
): azle.Result<boolean, string> {
    return azle.Result.Ok(
        record.star === concrete.star &&
            fakeRecord.star === fakeConcrete.star &&
            typeRecord.star === typeConcrete.star &&
            importedRecord.star === importedConcrete.star &&
            record.star === fakeRecord.star &&
            record.star === typeRecord.star &&
            record.star === importedRecord.star
    );
}

Here is the test I wrote for this. Obviously they need better names. I left all of the types in there because they aren't getting in the way and it will be easier then trying to tell you were everything goes. There is also a place holder compareStars function that you should replace with this one or your own better test. Maybe on that includes some more of the results examples I mentioned above

Next: In isIdentFromAzle() I have an if statement that will prematurely return false out of isIdentFromAzle(). That will need to be removed when you start working on this issue. Without it azle will pick up type aliases like those described in this ticket and try to include them in the generated lib folder. Since the developer can't control how library creators will make type aliases it seems possible that there is a bad type alias that will make things break that the user will have no control over. Granted that library developer should have a hard time compiling their library with azle if they have a type alias like that... so maybe it's not an issue, but I think it might be safer this way.

Here is another example I was working on. This is back when I thought that original example here was dealing with star exports, but now that I have thought through the problem a bit more, I think they are testing the same thing

// TODO can we have aliases like this? I mean imagine using azle.Result. We want
// that, but will it work?
// type MyMixedRecord = types.MixedRecordAlias<{
//     name: azle.text;
//     age: types.MixedUserDefinedAlias;
// }>;
// type MyStirredRecord = types.StirredRecordAlias<{
//     name: azle.text;
//     age: types.StirredUserDefinedAlias;
//     height: types.UserDefinedAlias;
// }>;

import {
    MixedUserDefinedAlias,
    UserDefinedAlias,
    StirredUserDefinedAlias
} from './types';

type MyMixedRecord = types.MixedRecordAlias<{
    name: azle.text;
    age: MixedUserDefinedAlias;
}>;
type MyStirredRecord = types.StirredRecordAlias<{
    name: azle.text;
    age: StirredUserDefinedAlias;
    height: UserDefinedAlias;
}>;

azle.$query;
export function addHeight(
    mixed: MyMixedRecord,
    height: azle.float64
): MyStirredRecord {
    return {
        name: mixed.name,
        age: { num: mixed.age.num },
        height: { num: height }
    };
}
lastmjs commented 1 year ago

Thinking through the problems with type 2 aliases, we think that the new Rust module system that tries to emulate the TypeScript module system including imports and exports, could be a solution to these problems. This is because we should have direct mappings of all imports and exports with the same naming conventions, thus the problems with azle.Result or as renaming should disappear.