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

feat: infer types from field initializers #15

Closed
wants to merge 2 commits into from

Conversation

bennypowers
Copy link
Member

See #12

@bennypowers
Copy link
Member Author

As this is still subject to review, I didn't update the fixtures yet, but I did commit the test output for your perusal

@@ -19,8 +19,16 @@ export function createField(node) {
/**
* Add TS type
* @example class Foo { bar: string = ''; }
*/
if(node.type) {
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets extract this into a separate handler (see handlers.js) that we can reuse in multiple places, something like handleTypeInference(fieldTemplate, node):

  switch(node?.initializer?.kind) {
    case ts.SyntaxKind.TrueKeyword:
    case ts.SyntaxKind.FalseKeyword:
      doc.type = { text: "boolean" }
      break;
    case ts.SyntaxKind.StringLiteral:
      doc.type = { text: "string" }
      break;
    case ts.SyntaxKind.NumericLiteral:
      doc.type = { text: "number" }
      break;
    case ts.SyntaxKind.NullKeyword:
      doc.type = { text: "null" }
      break;
    case ts.SyntaxKind.ArrayLiteralExpression:
      doc.type = { text: "array" }
      break;
    case ts.SyntaxKind.ObjectLiteralExpression:
      doc.type = { text: "object" }
      break;
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL

return v === 'true' || v === 'false';
}

export function isAsConst(initializer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does TS have other as somethings? I think we may need to think of a more generic approach of handling this, we collect types in a lot of other places as well. I want to avoid handling as const in one place, but not in another place.

Maybe we should separate handling as whatever in a different PR, and focus this PR on inferring types?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry i misunderstood the question, I read it as "does ts api have other isFoo`

as const is a special case, I'm not aware of any other keyword assertions

@thepassle
Copy link
Member

closed in favor of #53

I added you as co-author to that PR

We still need to adress as const, but lets do that separately now

@thepassle thepassle closed this Jun 30, 2021
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.

None yet

2 participants