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

Interface Index Signature Validation with Satisfies #28

Closed
Offroaders123 opened this issue May 11, 2023 · 1 comment
Closed

Interface Index Signature Validation with Satisfies #28

Offroaders123 opened this issue May 11, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@Offroaders123
Copy link
Owner

No description provided.

@Offroaders123 Offroaders123 added the bug Something isn't working label May 11, 2023
@Offroaders123 Offroaders123 self-assigned this May 11, 2023
Offroaders123 added a commit that referenced this issue May 11, 2023
Added support for using generics types with the NBTData class, and it's surrounding functional APIs!

Now if you know the shape of the NBT data going into the read function, you can pass that type into the read function generic parameter. This makes it so it can be a little easier to manage the types for the NBT structures going in and out of the library.

This is better than just plain type assertions on top of the `NBTData.data` property, because now the `NBTData` class type itself can hold both the NBT file metadata itself, and the shape of the data that it holds. I'm planning on using this to describe the full NBT file data structure and file metadata for world save data entries, so then you know things like the endian format, compression type, and what the actual NBT in the file is. So `NBTData` can be a full representation of the entire NBT file and how to make it with NBTify! This will work great once I eventually need to make NBT-generating classes which can make defaults for the these files when necessary. The class will simply implement the file's `NBTData` interface shape (including the NBT structure, file metadata), then the class will be fully type-safe with all of the information needed to make a fully symmetrical data tree to that of what's generated by the vanilla game :)

Currently, my use of validating the NBT data trees against the `CompoundTag` interface is currently typed too-strictly for what I'm trying to use it for, and it's forcing the interfaces and classes to require establishing index signatures, which are partially eliminating what I want the interface definitions to enable in the first place. I don't want my interfaces to allow the access and additions of unspecified properties, I only want to check that the values I am defining are compatible with the NBT primitive types.

microsoft/TypeScript#52222 (comment)
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-9.html

I created a new issue (#28) under NBTify to track the development of this typing issue in terms of how NBTify is handling this problem.
Offroaders123 added a commit that referenced this issue May 12, 2023
Removing the API-safe `CompoundTag` type checking for the library entry points, in the meantime. Without the index signature key value checking behavior that I need to accomplish the value type checking, there isn't another way to create API designs that will make the compiler happy, unfortunately. So this means I'm going to make all objects acceptable into NBTify, on the type level at least. Since I can't use type checking to validate the types for the shapes I'm passing in, I will just have to allow them in, and make sure that the shapes have appropriate value types.

While working on this revert, I looked into the `object` type, and realized it's kind of similar in a way as to what I'm trying to do with my primitive interface type checkers (`ListTag` and `CompoundTag`).

> Introduction to TypeScript `object` type
> The TypeScript `object` type represents all values that are not in primitive types.
>
> The following are primitive types in TypeScript:
>
> `number`
> `bigint`
> `string`
> `boolean`
> `null`
> `undefined`
> `symbol`

For my object definitions, I'm trying to define an object with keys that are only of a select few types. For the `object` type, it represents all JavaScript types that aren't a primitive type. So with this change, NBTify simply only checks if your `CompoundTag` is none of the primitive values mentioned above, which is better than just using `any` at least. It would be better if the check could also specify that your `CompoundTag` object cannot have things like `Function`s, `symbol`s, `undefined`, `null`, things like that. I think that's my main reason for wanting to add parameter type checking for your `CompoundTag` use.

https://www.typescripttutorial.net/typescript-tutorial/typescript-object-type/

microsoft/TypeScript#52222
(#28)
@Offroaders123
Copy link
Owner Author

Offroaders123 commented May 18, 2023

Kind of found a neat way to mitigate this, by manually adding a satisfies operator combination in conjunction with the data structure you'd like to make NBT from. (See it in this commit here 86aec8e)

Offroaders123 added a commit that referenced this issue May 19, 2023
Added support for using generics types with the NBTData class, and it's surrounding functional APIs!

Now if you know the shape of the NBT data going into the read function, you can pass that type into the read function generic parameter. This makes it so it can be a little easier to manage the types for the NBT structures going in and out of the library.

This is better than just plain type assertions on top of the `NBTData.data` property, because now the `NBTData` class type itself can hold both the NBT file metadata itself, and the shape of the data that it holds. I'm planning on using this to describe the full NBT file data structure and file metadata for world save data entries, so then you know things like the endian format, compression type, and what the actual NBT in the file is. So `NBTData` can be a full representation of the entire NBT file and how to make it with NBTify! This will work great once I eventually need to make NBT-generating classes which can make defaults for the these files when necessary. The class will simply implement the file's `NBTData` interface shape (including the NBT structure, file metadata), then the class will be fully type-safe with all of the information needed to make a fully symmetrical data tree to that of what's generated by the vanilla game :)

Currently, my use of validating the NBT data trees against the `CompoundTag` interface is currently typed too-strictly for what I'm trying to use it for, and it's forcing the interfaces and classes to require establishing index signatures, which are partially eliminating what I want the interface definitions to enable in the first place. I don't want my interfaces to allow the access and additions of unspecified properties, I only want to check that the values I am defining are compatible with the NBT primitive types.

microsoft/TypeScript#52222 (comment)
https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-9.html

I created a new issue (#28) under NBTify to track the development of this typing issue in terms of how NBTify is handling this problem.
Offroaders123 added a commit that referenced this issue May 19, 2023
Removing the API-safe `CompoundTag` type checking for the library entry points, in the meantime. Without the index signature key value checking behavior that I need to accomplish the value type checking, there isn't another way to create API designs that will make the compiler happy, unfortunately. So this means I'm going to make all objects acceptable into NBTify, on the type level at least. Since I can't use type checking to validate the types for the shapes I'm passing in, I will just have to allow them in, and make sure that the shapes have appropriate value types.

While working on this revert, I looked into the `object` type, and realized it's kind of similar in a way as to what I'm trying to do with my primitive interface type checkers (`ListTag` and `CompoundTag`).

> Introduction to TypeScript `object` type
> The TypeScript `object` type represents all values that are not in primitive types.
>
> The following are primitive types in TypeScript:
>
> `number`
> `bigint`
> `string`
> `boolean`
> `null`
> `undefined`
> `symbol`

For my object definitions, I'm trying to define an object with keys that are only of a select few types. For the `object` type, it represents all JavaScript types that aren't a primitive type. So with this change, NBTify simply only checks if your `CompoundTag` is none of the primitive values mentioned above, which is better than just using `any` at least. It would be better if the check could also specify that your `CompoundTag` object cannot have things like `Function`s, `symbol`s, `undefined`, `null`, things like that. I think that's my main reason for wanting to add parameter type checking for your `CompoundTag` use.

https://www.typescripttutorial.net/typescript-tutorial/typescript-object-type/

microsoft/TypeScript#52222
(#28)
Offroaders123 added a commit that referenced this issue Jul 17, 2023
Been looking into TypeScript's other tsconfig options, and I realized it would probably be of good use for me to try enabling more of the strict features that aren't already enabled with the 'strict' flag.

Was first looking into enabling 'noUncheckedIndexAccess', to which I then discovered one I hadn't heard of yet, 'noPropertyAccessFromIndexSignature'! This heavily helps with the NBT primitive type validation for the library's API parameter types, making use of the original index signature-based checks. This helps out so much, because it prevents you from accessing non-existant properties on the index signature-implemented structures, unless you use index-based accessing.

https://www.typescriptlang.org/tsconfig#noUncheckedIndexedAccess
https://www.typescriptlang.org/tsconfig#noPropertyAccessFromIndexSignature
https://www.npmjs.com/package/@tsconfig/strictest

#28
Offroaders123 added a commit that referenced this issue Jul 17, 2023
Further demos to distinguish valid and invalid class structures. This is much more strongly-typed now, yay! This is what I have been hoping for.

When I first thought of the NBT structure class builder idea, I was also considering having methods on the class too, but I don't think that makes sense when you think about it. I think I was trying to do too many things on the same level of the object. So I think this actually helps make the abstraction a bit simpler than I thought it should go, which is better. Having a mix of values/properties, and methods to work with that data, it doesn't really make sense for NBT representation. I want the class more to generate default values if you wanted to create a new NBT object representation from scratch, if I were to add that into the MC API. So, something like `new BedrockLevelDat()` would give you the same NBT object structure as a plain `level.dat` file, but with the defaults of a new world, unedited.

Come to think of it, I think this is kind of what I'm looking for, but I think using interfaces for the main NBT objects probably makes more sense, and to just use functions to make the objects, instead of classes. I could maybe see using classes for the `NBTData` representation of the file as a whole, but maybe that would suffer from the same issues, and it wouldn't make as much sense to build it like that. It could also make sense to use functions for that too, one for the internal `BedrockLevelDatLike` object for `NBTData.data`, and `BedrockLevelDat` for the `NBTData` wrapper, instead? That seems kind of like a neat idea too. It's a bit easier to do things at the type level too, rather than using objects, I can do union discriminant types for each of the platforms, and it can use NBTify's data primitives better too, since it won't use inheritance to describe the shapes of things.

#28
Offroaders123 added a commit that referenced this issue Jul 21, 2023
- noPropertyAccessFromIndexSignature
- noUncheckedIndexedAccess
- ListTag Generics

Discovered a few new entries that you can add to your tsconfig, and it actually partially (mostly) mitigates the issues with type-checked index signature-based parameter types!

Because the concern of using index signatures on interfaces would add haluceneted property keys that don't exist on the real structure (ones that you didn't specify), now you can only access real properties using dot notation, and if they are from an index signature definition only (not defined explicitly), then you have to access them using bracket notation.
#28

And a thing about that, I also added/re-looked into the config that ensures that you check that the values of your indexed-properties aren't undefined. This makes it so you have to ensure the items in your arrays, and things like that. Same with property names access like that too, I think.

```ts
const noice: string[] = [];
// could be undefined
const gg = noice[0];
```

Whilst thinking about how nice that is, I was wondering if you could use optional chaining with bracket notation, and turns out you can! Gonna try using that over in MCA-Buffer, once these changes roll out. I think it would work well for the implementation of a wrapper function for getting chunks at a specific x-y-z. I think it actually was thinking about the implementation for that which made me want to add these changes over to NBTify, to ensure the type-safety of things.
https://stackoverflow.com/questions/58780817/using-optional-chaining-operator-for-object-property-access

Now I just need users to ensure that they should probably use `noPropertyAccessFromIndexSignature` and `noUncheckedIndexedAccess` if possible (Now I feel like it could be an issue for those to be a requirement to use NBTify type-safely :(
Can you provide different types dependent on custom tsconfig settings? hmm. Maybe the workaround is just to use `xVar as CompoundTag` or `xVar as CompoundTag`)

```ts
interface CompoundTag extends Record<string,PropertyKey> {}
declare class Level {}

const level = new Level();
write(level as CompoundTag);

// Overload here which would use `object` only when `noPropertyAccessFromIndexSignature` or `noUncheckedIndexedAccess` can't be enabled?
declare function write(data: CompoundTag): Promise<Uint8Array>;
```

Brought back mandatory `ListTag` generics, as I want to help ensure that possible NBT type definitions will be as explicit as possible, and using a plain `ListTag` doesn't help as much with inferences. If you really want it to be any tag item type, then it should be an explicit choice to set it like that (which is what I do internally for the library now too, in places where it doesn't matter what the specific item type is). I'd rather the user set a more-narrow type for the definition, if they do know what kind it is. Any info like that would help out! And having an error for a missing generic will raise more eyebrows for needing to specify what type it is, than it jsut silently accepting the default "`any`" tag type.
@Offroaders123 Offroaders123 reopened this Sep 12, 2023
Offroaders123 added a commit that referenced this issue Sep 12, 2023
Trying another middle-ground solution to allow values that don't specifically extends the `RootTag` and friends' types. Essentially, the issue is that you can't use `as RootTag` on a plain object, and I don't want to force consumers to add `extends CompoundTag` or things like that to their shapes, because that will add unintended index signature key values to show up when accessing values off the type, which I discovered while working on Region-Types.

#19
#28
#32
Offroaders123 added a commit that referenced this issue Sep 13, 2023
```ts
type Compound = { [name: string]: boolean; };
type CompoundLike = object;

declare function read<T extends CompoundLike = Compound>(data: Uint8Array): Promise<T>;
```

Idea: Widen the scope of the generic parameter to allow more values which the user might like to pass in, but default to the more explicit value to check against! :D

#19
#28
#29
#32
Offroaders123 added a commit that referenced this issue Sep 13, 2023
This is definitely the most secure way to detect a default NBT value at the type level, and it allows you to provide your own interface to the function, and it doesn't have to inherit from `RootTag` and friends!

#19
#28
#29
#32
Offroaders123 added a commit that referenced this issue Sep 15, 2023
As with how I've been doing the last few updates, I make a some of the changes from the few previous updates since the last version release, so you can go to each commit about these topics specifically to get more info about what changed, and to see discussion about the process along the way too. I think trying to summarize it again just when I publish it isn't worth it compared to just referencing you towards the original write up about the changes I did the first time around. You'll get more explanations and such, I think it's more worth it :)

- ...and optional ListTag values
- RootTagLike (& RootTag default return types)
- Run-Time List Item Type Check (SNBT)
- Format Options Generic Reverting

#14
#19
#25
#28
#29
#31
#32
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

No branches or pull requests

1 participant