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

Model Schemas Not Generated Properly When Using Type Alases For Property Types #3863

Closed
taicho opened this issue Oct 3, 2019 · 4 comments
Closed
Assignees
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Docs

Comments

@taicho
Copy link
Contributor

taicho commented Oct 3, 2019

Steps to reproduce

A common pattern in TypeScript is to pull types from properties of other objects ensuring type safety and reducing redundancy...

  1. Create a Model or Entity:
@model({})
export class User extends Model {
    @property()
    public email : string;
    @property()
    public name : string;
    @property()
    public streetAddress : string;
    @property()
    public passwordHashedWithAPinchOfSalt : string;
}
  1. Create another with only a subset properties (maybe for use as a basic DTO for another endpoint):
@model({})
export class UserLogin extends Model {
    @property()
    public email : User["email"]; // --> When JSONSchema is generated this will resolve to an empty object type instead of string.
    @property()
    public password : string;
}

Current Behavior

As shown in the repro steps above, when pulling property types from other object types an incorrect JSON Schema is formed and validation on that property is no longer enforced due to the type not being inferred correctly.

Expected Behavior

In the example above, since User.email is of type string, it would be expected that the resolved type of UserLogin.email should be the same e.g. type string.

Sidenote. A "workaround" for this is to manually specify the type in the @property decorator options but this is less than ideal, defeats the purpose of eliminating redundancy, and I would argue unexpected.

Additional information

  • darwin x64 10.16.0

├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── @loopback/[email protected]
├── [email protected]

Related Issues

None found upon basic search.

@taicho taicho added the bug label Oct 3, 2019
@bajtos
Copy link
Member

bajtos commented Oct 4, 2019

I am afraid this is a limitation of TypeScript.

Here is the code emitted for your UserLogin class:

let UserLogin = class UserLogin extends __1.Model {
};
__decorate([
    __1.property(),
    __metadata("design:type", Object)
], UserLogin.prototype, "email", void 0);
__decorate([
    __1.property(),
    __metadata("design:type", String)
], UserLogin.prototype, "password", void 0);
UserLogin = __decorate([
    __1.model()
], UserLogin);

As you can see on the line 3, TypeScript is providing the following design:type metadata for the email property: Object. LoopBack is using this metadata to infer the property type, because that's the only information available at runtime. And because TypeScript thinks the type is Object, LoopBack defines the property as object too.

Feel free to search TypeScript issue tracker to see if this is a known problem and perhaps open a new issue if it's not.

As for LoopBack, I think our best option is to improve the documentation to make this fact easier to find by users. Do you have any suggestions where would you look for such information yourself?

Would you like to contribute the doc changes yourself? See our Contributing guide and Submitting a pull request to LoopBack 4 to get started.

@bajtos bajtos added developer-experience Issues affecting ease of use and overall experience of LB users Docs and removed bug labels Oct 4, 2019
@taicho
Copy link
Contributor Author

taicho commented Oct 4, 2019

Ah. I'm relatively new to LB4 - I assumed the OpenAPI spec was getting generated via TS API by parsing the AST rather than reflection metadata, that makes sense. Also interesting that the injected metadata doesn't handle this situation on the TypeScript side, definitely seems like a gap in the compiler.

In any case, @bajtos thanks for the quick reply and breakdown. My best guess to provide footnote/warning/caveat would be in Key Concepts->Models. That being said, I'll look at contributing the change to the documentation on this side and the issue on the TypeScript side. Thanks!

taicho added a commit to taicho/loopback-next that referenced this issue Oct 4, 2019
Changes Model.md to reflect a known issue with property reflection when dealing with type aliases and extracted types. See loopbackio#3863. No code changes.
taicho added a commit to taicho/loopback-next that referenced this issue Oct 5, 2019
Changes Model.md to reflect a known issue with property reflection when dealing with type aliases and extracted types. Edit - adds example suggested by @raymondfeng.  See loopbackio#3863. No code changes.
taicho added a commit to taicho/loopback-next that referenced this issue Oct 5, 2019
Changes Model.md to reflect a known issue with property reflection when dealing with type aliases and extracted types. Edit - adds example suggested by @raymondfeng.  See loopbackio#3863. No code changes.
taicho added a commit to taicho/loopback-next that referenced this issue Oct 5, 2019
Changes Model.md to reflect a known issue with property reflection when dealing with type aliases and extracted types. Edit - adds example suggested by @raymondfeng.  See loopbackio#3863. No code changes.
@dhmlau
Copy link
Member

dhmlau commented Oct 7, 2019

@taicho, i've assigned this issue to you because you have a PR #3871 in progress

raymondfeng pushed a commit that referenced this issue Oct 7, 2019
Changes Model.md to reflect a known issue with property reflection when dealing with type aliases and extracted types. Edit - adds example suggested by @raymondfeng.  See #3863. No code changes.
@taicho
Copy link
Contributor Author

taicho commented Oct 7, 2019

Closing this as #3871 has been merged.

@taicho taicho closed this as completed Oct 7, 2019
@dhmlau dhmlau added this to the Oct 2019 milestone milestone Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Docs
Projects
None yet
Development

No branches or pull requests

3 participants