Loopback-next: [Model discovery] Generated code does not seem to be correct

[dhmlau]

Description / Steps to reproduce / Feature proposal

Using MySQL as the datasource, I created a table called Customer. Command to create the table is as below:

create table customer (custid VARCHAR(100), name VARCHAR(100));
ALTER table customer add primary key(custid);

The generated customer model looks like:

   @model([object Object])
   export class Customer extends Entity {
     @property({
       type: String,
       required: true,
       length: 100,
       id: 1,
       mysql: {"columnName":"custid","dataType":"varchar","dataLength":100,"dataPrecision":null,"dataScale":null,"nullable":"N"},
     })
     custid: String;

     @property({
       type: String,
       required: false,
       length: 100,
       mysql: {"columnName":"name","dataType":"varchar","dataLength":100,"dataPrecision":null,"dataScale":null,"nullable":"Y"},
     })
     name?: String;
     ...
   }
   ```

There are a few issues in the code:
1. Model decorator is not correct: `@model([object Object])`
2. For the string type, should it be `string` instead of `String`?
3. Inside the property decorator, should the type be `type: 'string',` instead of `type:String`

### Acceptance Criteria
Fix the generated code (@bajtos had some pointers for each of them, see https://github.com/strongloop/loopback-next/issues/2953#issuecomment-495578669): 
- [ ] Model decorator is not correct: `@model([object Object])`
- [ ] For string type, it should be `string` instead of `String`
- [ ] Inside the property decorator, it should be `type: 'string',` instead of `type:String`
- [ ] For connector specific options, it should be represented as a regular TypeScript object, i.e. for 

mysql: {"columnName":"name","dataType":"varchar","dataLength":100,"dataPrecision":null,"dataScale":null,"nullable":"Y"},
})
it should be: {
@property({
// ...
mysql: {
columnName: 'name',
dataType: 'varchar',
dataLength:100,
dataPrecision:null,
dataScale:null,
nullable: 'Y'
},
})
name?: String;
}```

[dhmlau]

cc @marvinirwin

[bajtos]

Model decorator is not correct: @model([object Object])

Are we perhaps calling .toString() instead of converting the object to JSON or TypeScript? Let's use the same approach I introduced for lb4 model in https://github.com/strongloop/loopback-next/pull/2843.

For the string type, should it be string instead of String?

Yes please. While String works too, it's considered as a bad practice in TypeScript.

@model([object Object])
export class Customer extends Entity {
  @property({
    // ...
  })
 custid: string;

  // ...
}

Inside the property decorator, should the type be type: 'string', instead of type:String

Juggler supports both forms. For consistency with the rest of LB4, it would be better to use type: 'string'.

I also noticed that connector-specific options are represented as JSON:

{ 
  @property({
    // ...
    mysql: {"columnName":"name","dataType":"varchar","dataLength":100,"dataPrecision":null,"dataScale":null,"nullable":"Y"},
     })
     name?: String;
}

It would be nice to represent them as a regular TypeScript object, similarly to what I introduced in https://github.com/strongloop/loopback-next/pull/2843 for model settings.

{ 
  @property({
    // ...
    mysql: {
      columnName: 'name',
      dataType: 'varchar',
      dataLength:100,
      dataPrecision:null,
      dataScale:null,
      nullable: 'Y'
    },
  })
  name?: String;
}

[redaikidoka]

I'd like to add on that the @model([object Object])
does not compile, generating:
ERROR TS1005

It can be corrected by outputing @model() instead.

[gonadarian]

Sorry for jumping in as I'm a complete novice to Loopback, but...

I generated a model (can't remember how :disappointed:) a few days ago (don't know the version as I updated to the latest just now :disappointed:) which had no compile errors with this code @model({settings: {}}). Now I'm also getting @model([object Object]).

Looks like a regression, but can't be sure...

[dhmlau]

@gonadarian, thanks for the info. It's good to know. @bajtos has a suggestion on how to fix @model([object Object]). If you're interested in submitting a patch, please feel free!

Are we perhaps calling .toString() instead of converting the object to JSON or TypeScript? Let's use the same approach I introduced for lb4 model in #2843.

[marvinirwin]

Sorry I'm late, If nobody else has started I'll try and create the fix PR in a day or two.

[dhmlau]

@marvinirwin, sure, that would be great.
I have another question for you.. Could you please take a look at my comment https://github.com/strongloop/loopback-next/issues/2915#issuecomment-495450082? I'm thinking that --schema option is needed, and we'll need to update the docs. Please let me know if it's not the case. Thanks!

[bajtos]

Now I'm also getting @model([object Object]).

That look super weird to me! In https://github.com/strongloop/loopback-next/pull/2843, I changed the code emitting @model to always stringify the model options into JavaScript code. It's entirely possible that I missed an edge case that does not work now, how can I reproduce the problem you are experiencing?

When I run lb4 app and lb4 model, then I see the following file being scaffolded in src/models/product.model.ts

import {Entity, model, property} from '@loopback/repository';

@model({settings: {}})
export class Product extends Entity {

  constructor(data?: Partial<Product>) {
    super(data);
  }
}

[bajtos]

I think this issue will be fixed by #3015.

[samarpanB]

I can confirm that this issue is fixed with latest cli version. Just tried with 1.16.1. But there is a different issue coming up on discovery. I'll raise that separately.

[bajtos]

Thank you @samarpanB for the confirmation. I am closing this issue as fixed then.