Publish ambient types #77
Conversation
gitKrystan
force-pushed
the
ambient-types
branch
2 times, most recently
from
a373404 to
284e7d9
Compare
| @@ -0,0 +1,18 @@ | |||
| export default class TestLoader { | |||
There was a problem hiding this comment.
Based on the docs for this library, I'm not sure how much of this should actually be public. All I need for emberjs/ember-qunit#994 is loadModules, moduleLoadFailure, addModuleExcludeMatcher and addModuleIncludeMatcher.
There was a problem hiding this comment.
Should we comment out the rest and see if anyone asks for them?
There was a problem hiding this comment.
I guess at the end of the day this whole package is probably pretty "private" anyway 🤷🏼♂️
There was a problem hiding this comment.
Ok so looking at the implementation, I would guess that shouldLoadModule, listModules, listTestModules, require, unsee and moduleLoadFailure are probably meant to be private helpers.
There was a problem hiding this comment.
In that case, it may make more sense to declare the default export as an object with a load method on it? I don't think TestLoader is really meant to be instantiated or subclassed by anyone outside of here. The fact that it happens to be a class appears to be an implementation detail.
There was a problem hiding this comment.
OTOH, the readme says....
// optionally override TestLoader.prototype.shouldLoadModule
🤷🏼 🤷🏼 🤷🏼
I guess you can take all of that information and decide what to do with them as you see fit, this is just an "internal" support package and I don't think these things were thought through very rigorously at the time.
There was a problem hiding this comment.
ember-qunitdoesnew TestLoader().loadModules()
It probably should have just called load() which does exactly that, but oh well
There was a problem hiding this comment.
README says "optionally override TestLoader.prototype.shouldLoadModule" so I am going to make that public also
There was a problem hiding this comment.
I am working on ember-qunit anyway, so I just made that call the static load method and made loadModules private and the constructor protected.
gitKrystan
force-pushed
the
ambient-types
branch
3 times, most recently
from
c021e6e to
68d52ec
Compare
gitKrystan
force-pushed
the
ambient-types
branch
from
68d52ec to
d1a6026
Compare
4 participants
Needed for emberjs/ember-qunit#994
See emberjs/ember.js#20162