Confusing @model() syntax

[bajtos]

In LB3, users could specify model settings at two levels: as a root property or inside options property.

See https://stackoverflow.com/q/53307168/69868 for an example of LB3 syntax applied in an LB4 project:

@model({
  settings: {strict: false},
  name: 'client',
  plural: 'clients',
  options: {
    mongodb: {
      collection: 'clients',
    },
  },
})
export class Client extends Entity {
  // ...
}

I am proposing to make two changes in LB4 to help users coming from LB3:

1. Recognize options the same way as settings. In the example above, mongodb settings are not picked by LB4 now. With the proposed change in place, LB4 will set collection to clients as expected. Alternatively, tell the user setting options that they are trying to set an unsupported model-definition property. This can be done at compiler level too.

2. Allow settings to be provided as top-level properties, for example:

   @model({
     name: 'client',
     strict: false,
     mongodb: {
       collection: 'clients',
     },
   })
   export class Client extends Entity {
     // ...
   }

[samarpanB]

@bajtos If its fine, I can try help with this feature implementation. Let me know if that's fine ? I have not worked with LB3, so I may need some guidance on exact requirements.

[bajtos]

@samarpanB of course it's fine to contribute! Sorry for the delay, I am often overwhelmed by GitHub notifications and I am still learning how to organize them so that I can focus on more important matters first. Shout out to https://octobox.io which helps me a lot!

I have not worked with LB3, so I may need some guidance on exact requirements.

Here is the code that's "normalizing" model configuration in LoopBack 3.x:

https://github.com/strongloop/loopback/blob/9b1d4885ab869d977bc7d464aa861b4f88691862/lib/registry.js#L207-L227

  // Settings
  var excludedProperties = {
    base: true,
    'super': true,
    relations: true,
    acls: true,
    dataSource: true,
  };
  if (typeof config.options === 'object' && config.options !== null) {
    for (var p in config.options) {
      if (!(p in excludedProperties)) {
        settings[p] = config.options[p];
      } else {
        g.warn('Property `%s` cannot be reconfigured for `%s`',
          p, modelName);
      }
    }
  } else if (config.options != null) {
    g.warn('The options property of `%s` configuration ' +
      'must be an object', modelName);
  }

Essentially, we want to recognize few reserved top-level keys like name, relations, settings, options, these have special treatment. All other top-level properties (e.g. strict) are treated as model settings and copied to the settings object.

[samarpanB]

Thanks for the update @bajtos . I am right now running little too busy with other projects. I'll start on it as soon as I get those completed. May be in 2-3 weeks. If anybody else is willing to pick this up from community, feel free to assign.

[bajtos]

No worries @samarpanB, take your time!

On the second look, I think I posted a wrong LB3 code in my previous comment. Here is the correct one:

https://github.com/strongloop/loopback/blob/9b1d4885ab869d977bc7d464aa861b4f88691862/lib/registry.js#L130-L147

function buildModelOptionsFromConfig(config) {
  var options = extend({}, config.options);
  for (var key in config) {
    if (['name', 'properties', 'options'].indexOf(key) !== -1) {
      // Skip items which have special meaning
      continue;
    }


    if (options[key] !== undefined) {
      // When both `config.key` and `config.options.key` are set,
      // use the latter one
      continue;
    }


    options[key] = config[key];
  }
  return options;
}

[bhupesh-sf]

@samarpanB if you are going to take sometime on this can I meanwhile take this up?

[samarpanB]

Yes, please. Feel free to take it up @bhupesh-sf . @bajtos you can assign it to @bhupesh-sf if you see its fine.