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

Missing type check on field initializer when exporting a class without constrtuctor #1150

Closed
saulecabrera opened this issue Mar 9, 2020 · 1 comment · Fixed by #1154
Closed
Labels

Comments

@saulecabrera
Copy link
Contributor

While working on #482 I found out that when an exported class without constructor defines a property with an explicit initializer there's no explicit typecheck.

Reproduce

  • Export a class
  • Do not define a constructor
  • Add a field with an incorrect initializer
export class Foo {
  a: i32 = "44";
}

The previous example compiles without errors.

Expected behaviour

The expectation would be to report a type error back, just like when the class defines a constructor:

export class Foo {
  a: i32 = "44";
  constructor() {}
}
ERROR TS2322: Type '~lib/string/String' is not assignable to type 'i32'.

Analysis

From my investigation, this happens only in exported class definitions without constructor because the compilation path for an exported class is slightly different than an instantiation.

When compiling an instantiation, if the class doesn't define a constructor a default one is created, on the other hand, when compiling a class export with no constructor, field initialization is never triggered and consequently, no type errors are reported back.

Should a default constructor be created in this case as well? Or are there any drawbacks with this?

@dcodeIO
Copy link
Member

dcodeIO commented Mar 9, 2020

Should a default constructor be created in this case as well?

Triggering a default constructor in this scenario (a class with instance members but no constructor is exported) sounds reasonable to me, since a class without a constructor cannot have instances and without instances the export doesn't make sense.

Not quite sure what to do with edge cases like classes that only contain static members. On a first glimpse it seems unnecessary to trigger a default constructor there. However, when following this argument to the end, if there is any other exported element dealing with an instance of that class, there should still be a constructor because otherwise the other element export doesn't make sense again. Hmm...

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

Successfully merging a pull request may close this issue.

2 participants