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

add generic interface support for tableSchema function #1437

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

onursagir
Copy link

Added an optional generic for the tableSchema function this allows for project using Typescript to show mistakes when creating a schema for example:

interface Test {
  name: string;
}

tableSchema<Test>({
  name: 'test',
  columns: [{ name: 'name', type: 'number' }], // Type '"number"' is not assignable to type '"string"'
});

Alos forces you to provide isOptional: true if the property is nullable

interface Test {
  name: string;
  age?: number;
}

tableSchema<Test>({
  name: 'test',
  columns: [
    { name: 'name', type: 'string' },
    { name: 'age', type: 'number' }, // Property 'isOptional' is missing in type ...
  ],
});

@onursagir
Copy link
Author

Alternatively one could go with an approach like this. Which is nice because it would require the user to provide all the keys present in the interface however one downside is that it would also require that they are provided in reverse order. Let me know what you think I can update the pr accordingly

@radex
Copy link
Collaborator

radex commented Jan 9, 2023

@onursagir Looks neat! Is it required to provide the type parameter after this change? In other words, will existing code continue to work without typing out table schemas?

@onursagir
Copy link
Author

@radex No I don't think so, the type falls back to unknown if none is provided.

@radex
Copy link
Collaborator

radex commented Jan 9, 2023

@onursagir In this case I'm cool with merging it, but please add a note to CHANGELOG-Unreleased

@onursagir
Copy link
Author

@radex which variant would you prefer

  • the one in the in the sandbox link is stricter requiring alls the keys in the properties to also be present in the object but has a downside that the order of the properties should also match the order in the interface
  • the one in the pr is looser allowing you to omit properties (which is a downside imo) and declare the same properties twice but isn't strict on order

@radex
Copy link
Collaborator

radex commented Jan 9, 2023

@onursagir Stricter is better IMO

@onursagir
Copy link
Author

@radex I'll update this pr over the weekend accordingly I will ping you when it's ready again

@onursagir onursagir closed this Jan 9, 2023
@onursagir onursagir reopened this Jan 9, 2023
@radex
Copy link
Collaborator

radex commented Jan 20, 2023

@onursagir Hey, any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants