Find replacement for DI

[dignifiedquire]

The di (dependency injection) module we currently use is very outdated and the di setup we have is pretty hard to understand, especially for people new to the code base.

Currently all plugins though depend on it, so we need to find a backwards compatible solution or at least an interface that is as simple to use as the current one for plugin authors.

[maksimr]

IMHO

I really like idea of dependency injection(DI) .
Only one thing in karma's DI like a magic. It is DI using comment in function's argument:

fooReporter(/*config.client*/client) { return client }

@dignifiedquire why do you want replace it? Only because people can not understand it?
Really they don't understand it or maybe we could improve DI's API make it more obvious if it's not ovious now

✌️

[dignifiedquire]

@maksimr I am not sure, maybe just update things to use a newer library. If we are happy with DI itself I am happy to just move to a maintained library.

[EzraBrooks]

After doing a short review of how often we actually use di, it seems like it's not really a requirement for the project and could probably be refactored out. I think refactoring it out would make the whole application be less "magic" and probably help with static analysis (via tools like TypeScript).

[EzraBrooks]

Main issue is, as you said, refactoring it out of Karma without breaking things for plugin authors.

[EzraBrooks]

@dignifiedquire would you be open to a PR utilizing injection-js? Could also combine that feature with moving from pseudo-classes to proper ECMAScript classes, since that should make injection easier. Currently my TypeScript PR (#2893) is blocked by a bunch of ambiguous typing that is a result of pseudo-classing making things unclear.

[johnjbarton]

I'm curious about your definition of pseudo-classes. Are you talking about code like url

karma/lib/url.js

Line 11 in 39d378d

Url.prototype.toString = function () {

which defines methods on .prototype (should be converted to es6 classes) or like dots

karma/lib/reporters/dots.js

Line 13 in 39d378d

this.onBrowserStart = function (browser) {

where methods are defined directly on the objects (where changes may have broader consequences)?

Or both ;-)

[EzraBrooks]

Both, although the former is more egregious. I was wrestling with type ambiguity in file-list the other night and the easiest solution was rewriting the whole thing as an ES6 class.

[dignifiedquire]

• I would love to see the code base upgraded to use class, const, let and arrow functions.
• I am very open to use a different di library, as long as we can keep all existing plugins compatible. There are a lot of plugins out there, from various authors which rely on the current workings to work. Maintained + tested is the base requirement. But I would like to stay away from compiling things, so ideally using the regular es6 version, no typescript class annotations

[johnjbarton]

I suggest tackling es6 and new DI separately, starting with a simple case, and developing a documented recipe for these changes. I guess the test coverage for the plugin space is not sufficient to ensure that a passing karma test will ensure plugins work. So patience and documentation will be essential.

[EzraBrooks]

Does the old DI work with new class syntax? My concern is that rewriting them separately might raise unexpected compatibility issues and it might just be best to make any breaking changes simultaneously instead of in multiple steps.

[johnjbarton]

es6 classes can be used with es3 code; it may or may not be pretty and some investigation may be needed.

Changing the DI is, I believe, a much bigger issue because it is part of how plugins interface with karma. We would need some kind of migration strategy where old and new DI are both allowed, then we reach-out/DIY the changes for the top plugins, then deprecate the old DI.