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

[BUG] Typescript bindings - Options, CustomEnums, fieldorder #2784

Closed
starknetdev opened this issue Dec 9, 2024 · 1 comment · Fixed by #2816
Closed

[BUG] Typescript bindings - Options, CustomEnums, fieldorder #2784

starknetdev opened this issue Dec 9, 2024 · 1 comment · Fixed by #2816
Labels
bug Something isn't working

Comments

@starknetdev
Copy link
Contributor

Describe the bug
Some issues remaining in the typescript bindings on the back of CairoOption and CustomEnum PR.

To Reproduce

  • Clone ls-tournaments
  • run, cd tournaments && cd contracts && sozo build --typescript
  • inspect, contracts/bindings/typescript/models.gen.ts + contracts.gen.ts

or find the file here:

CairoOption

  • contracts.gen.ts references CairoOption in the models file, rather than using the starknet import.
  • GatedType is not being referenced in the model file.
  const tournament_mock_createTournament = async (
    ...
    gatedType: models.CairoOption<GatedType>,
    entryPremium: models.CairoOption<GatedType>
  ) => {
   ...
  };

Cairo Options aren't defined properly in the schema.

		TournamentModelValue: {
                        ...
			gated_type: Option,
			entry_premium: Option,
		}

I have defined them as so (still questioning exactly how this would relate to the torii types as they are different)

      gated_type: new CairoOption(CairoOptionVariant.None),
      entry_premium: new CairoOption(CairoOptionVariant.None),

CustomEnum

Not being defined properly in the schema.

		TokenModelValue: {
		       ...
			token_data_type: { fieldOrder: ['token_amount'], token_amount: 0, },
		}

I have defined as

    TokenModelValue: {
     ...
      token_data_type: {
        variant: {
          fieldOrder: ["erc20", "erc721"],
          erc20: { fieldOrder: ["token_amount"], token_amount: 0 },
          erc721: undefined,
        },
        unwrap: function () {
          const variants = Object.values(this.variant);
          const value = variants.find((v) => v !== undefined);
          return value;
        },
        activeVariant: () => "erc20",
      },
    },

Some custom enums aren't being defined properly

// Type definition for `tournament::ls15_components::models::tournament::GatedEntryType` enum
export enum GatedEntryType {
	criteria,
	uniform,
}

Expected

// Type definition for `tournament::ls15_components::models::tournament::GatedEntryType` enum
export type GatedEntryType = {
  criteria: EntryCriteria[];
  uniform: number;
};
export type GatedEntryTypeEnum = TypedCairoEnum<GatedEntryType>;

Fieldorder

It seems the new RemoveFieldOrder doesn't account for field orders in some of the new types. I have extended the removal to these in my implementation.

Before

type RemoveFieldOrder<T> = T extends object
  ? Omit<
      {
        [K in keyof T]: T[K] extends object ? RemoveFieldOrder<T[K]> : T[K];
      },
      'fieldOrder'
    >
  : T;

After

type RemoveFieldOrder<T> = T extends object
 ? T extends CairoOption<infer U>
   ? CairoOption<RemoveFieldOrder<U>>
   : T extends CairoCustomEnum
   ? T
   : Omit<
       {
         [K in keyof T]: T[K] extends object ? RemoveFieldOrder<T[K]> : T[K];
       },
       "fieldOrder"
     >
 : T;

Since removing the field orders for nested types is quite messy, perhaps we could do the reverse and add the field orders only for the model schemas as suggested here #2748 (comment)

@starknetdev starknetdev added the bug Something isn't working label Dec 9, 2024
@MartianGreed
Copy link
Contributor

gm ser,

Let me check this out !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants