Should we introduce sugar for bulk app bindings?

[shimks]

Currently, Applications can only bind their components, controllers, and repositories through calling app.component, app.controller, and so forth. If a user wanted to bind multiple components, they would have to bind them one by one like so:

app.component(FooComponent);
app.component(BarComponent);
app.component(BazComponent);
app.component(RestComponent);

With #742, sugar for accepting array of components were supposed to be added but with boot, the sugar may not be necessary anymore.

What is our consensus on this issue?

• connected to #742, #975

[shimks]

https://github.com/strongloop/loopback.io/pull/619#pullrequestreview-96401594

• get rid of already-existing servers binding function

[raymondfeng]

Personally, I like the REST parameter flavor:

app.controller(...controllerCtors);

But I don't mind calling the method one by one for each controller as it won't be used by application code so often.

app.controller(c1);
app.controller(c2);

The above call can be simplified as:

[c1, c2].forEach(c => app.controller(c));

[kjdelisle]

I don't mind having an arbitrary args list for things (app.controllers(...controllerCtors)), but I would say that I do mind having to add potentially dozens of controllers to my app context one line at a time!

I know that our boot module is going to take a lot of the manual involvement out of typical wireup, but it's a small amount of effort to provide this sugar for users.

That being said, I would like the sugar method to be different (app.controllers(...controllerCtors) vs app.controller(controller)), and for the plural version to leverage the singular version (simplifies tests and lets other code use the singular version to limit testing requirements/concerns)

[virkt25]

I think getting rid of the optional controller name and just switching to the Rest flavour as proposed by Raymond would be my recommendation as well. I don't think we need a different sugar method with a s at the end for that since the one method can take one or N many Controller Classes.

app.controller(MyController)
app.controller(MyControllerOne, MyControllerTwo, MyControllerThree)

[raymondfeng]

Here is what we can do to support all styles in one signature:

/**
 * Normalize array or rest parameters into a flat array of arguments
 * @param params An array or rest parameters of values. For example,
 * - tag('t1') // A single param
 * - tag('t1', 't2') // Rest params
 * - tag(['t1', 't2']) // A param with array value
 * - tag('t1', ['t2', 't3'], 't4') // Mixed usage of values and arrays
 */
export function getArrayOrRestArgs<T>(...params: (T | T[])[]): T[] {
  const args: T[] = [];
  for (const p of params) {
    if (Array.isArray(p)) {
      args.push(...p);
    } else {
      args.push(p);
    }
  }
  return args;
}

[dhmlau]

i don't think it should be part of MVP. What do you think? @raymondfeng @bajtos @strongloop/sq-lb-apex

[bajtos]

+1 for leaving this out of MVP.

I am personally fine with the current API allowing only a single value and requiring users to call e.g. app.controller multiple times. I expect very few people to call app.controller manually.

I am strongly against having two methods with names that differ only in the singular/plural form (app.controller vs. app.controllers), because such names are so easy to confuse when reading code. If we decide to provide two methods then the plural one should use a distinctly different name, e.g. registerManyControllers (maybe something less silly).

[dhmlau]

@raymondfeng @shimks @virkt25, @bajtos proposed to close it. What do you think?

[virkt25]

I'd be ok to leave it out of the scope of DP3 (but it's a simple enough experience) ... but I would like for us to implement the ...rest flavour for binding in at least app.component since there is no way we can support components from @loopback/boot as they are npm modules people will install. And for consistency in our API I would recommend all binding methods support this flavour (but I'll be fine if they don't).

[raymondfeng]

+1 to support the rest params.

[bajtos]

I would like for us to implement the ...rest flavour for binding in at least app.component since there is no way we can support components from @loopback/boot as they are npm modules people will install.

FWIW, loopback-boot already supports loading components - see https://loopback.io/doc/en/lb3/component-config.json.html.

IMO, the fact that components are npm modules should not prevent us from loading them automatically based on conventional configuration, as long as there is a common API for components/extensions.

In LoopBack3, the API contract for an extension is to provide a default export that's a function accepting app and options. Obviously, LB4 has a different design, but why cannot @loopback/boot call app.component(require('extension-name'))?

[virkt25]

We can definitely have a booter for components if we are given a list of components / node_modules to require the components from ... I thought we were talking about traversing the node_modules folder and trying to identify Components ourselves and loading them.

And for custom components in a components folder ... we can definitely just boot those even without a list.

[raymondfeng]

Now we have https://loopback.io/doc/en/lb4/Binding.html#configure-binding-attributes-for-a-class, it makes more sense to allow rest parameters, such as:

app.component(component1, component2, ...);
app.controller(controller1, controller2);

[bajtos]

Now we have https://loopback.io/doc/en/lb4/Binding.html#configure-binding-attributes-for-a-class, it makes more sense to allow rest parameters

+1

I think we should also preserve the current API that allows callers to customize the binding without the need to use decorators - this is important for JavaScript consumers for example.

app.controller(DefaultCrudCntroller, 'MyControllerName');
app.repository(RepoClass, 'UserRepository');
app.dataSource(DataSourceClass, 'db');

It would be great if somebody can compile a list of APIs (methods) we need to change as part of this story and create a check-list in the acceptance criteria in the issue description. Places where t look:

• Application, RestServer and any other classes inheriting from Context
• RepositoryMixin, BootMixin, ServiceMixin and other mixins

[stale[bot]]

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

[stale[bot]]

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

[stale[bot]]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.