Implement knownForType for RFC#58.

[rwjblue]

Supports emberjs/ember.js#11393.

Further reading: emberjs/rfcs#58.

[rwjblue]

I split this into two commits to make it easier to review. The first commit is the actual change, and the second is updating dist/.

[stefanpenner]

Ember.keys

[stefanpenner]

i believe in the past the idea was Array.isArray(resolveAllForType(name)) === true, any reason why we instead opted for the current iterator style?

The array form is more flexible, especially in light of the fun iterables have in es6+

[rwjblue]

@stefanpenner - I don't fully understand your suggestion. Can you elaborate? What would 'resolveAllForType' return (or do)?

[rwjblue]

The current primary use-case for this is to enumerate the list of names for the current type for the dashless helper RFC. In that case we actually do not have a use for the factories (since we are just whitelisting the helper names themselves), but I added them here because I think we will find that useful in the future.

I could definitely see this method returning an object instead of calling a callback if you would prefer that, but I don't see how an array would work (since we need both a key and a value).

[stefanpenner]

@stefanpenner - I don't fully understand your suggestion. Can you elaborate? What would 'resolveAllForType' return (or do)?

• So this feature looks useful for more then just dashless helpers, but also initializers and instance initializers.
• Also, rather then locking us into having to use forEach to enumerate, by returning an array (instead of just exposing forEach), We can also filter/map/inject/for in/for on the collection. Basically it just feels more flexible.

[rwjblue]

@stefanpenner - To confirm, you would prefer something like the following:

var items = resolver.eachForType('fruit');

/*
  Where items is something like:
  {
    'fruit:apple': AppleFactory,
    'fruit:orange': OrangeFactory
  }
*/

Right?

[teddyzeenny]

Looks similar to what container-debug-adapter does. Should we consider removing the container debug adapter?

[rwjblue]

@teddyzeenny - I think that could be refactored to use the new method here, but I'm not sure that it completely replaces the container debug adapter.

[stefanpenner]

Right?

i was actually just thinking an array of names, but an array of well formed pojos is also really cool

[rwjblue]

@stefanpenner - Updated to address your concerns (as I understood them):

• Swapped to using Ember.keys instead of for/in.
• Remove callback argument so a plain dictionary is returned.

Can you double check, and 👍 / 👎?

[stefanpenner]

Remove callback argument so a plain dictionary is returned.

as mentioned above, an array of well formed pojos seems like the must flexible return value.

[rwjblue]

@stefanpenner - Not sure that I agree.

How is an array of key/value objects better than an actual structure that is built to deal with keys and values (a single object)?

[
  {key: 'fruit:apple', value: AppleFactory},
  {key: 'fruit:orange', value: OrangeFactory}
]

vs

{
  'fruit:apple': AppleFactory,
  'fruit:orange': OrangeFactory
}

Are you concerned with needing to add more things than key (container fullname) and value (actual factory)?

[rwjblue]

In the end I just want to settle this so we can move forward on the dashless helpers RFC which needs to be out in 1.13 tomorrow.

If you are certain that you want an array of objects, I will change to that API.

[stefanpenner]

How is an array of key/value objects better than an actual structure that is built to deal with keys and values (a single object)?

the common pattern is to enumerate/filter/map/transform this information. It seems like exposing an iterable primitive is appropriate.

[stefanpenner]

helpers RFC which needs to be out in 1.13 tomorrow.

this is true, but once we land this it will be around for ages, so being mindful and diligent now saves us from prolonged pain

[rwjblue]

this is true, but once we land this it will be around for ages, so being mindful and diligent now saves us from prolonged pain

Agreed, but please remember that this is also a private API used only by Ember itself ATM.

[rwjblue]

Updated to use an array of objects as requested.

[stefanpenner]

hmm, your likely going to be frustrated by this, but I just realized these iterables may yield duplicates and we have lost the precedence provided during resolving.

for example:

app/components/edit-user.js
app/pods/edit-user/componeont.js

clearly in resolving, we realize the later shadows the former.

[rwjblue]

Yes, using the object form automatically de-dupes for us and allows us to ensure the proper ordering.

If your primary concern with the object case is future expansion in what the values should be (for example maybe we also want modulePath), I could make something like:

{
  'fruit:apple': {key: 'fruit:apple', value: AppleFactory},
  'fruit:orange': {key: 'fruit:orange', value: OrangeFactory}
}

That still uses an object for de-duping and prioritization, but allows expansion on the number of keys in the object later...

[stefanpenner]

@rwjblue the object form clearly dedupes, but I am unclear where in the code it takes the priority into account

[rwjblue]

This PR only addresses the resolver parts, not registry or fallback registry. That is done in the Ember PR which will have the order (later entries overriding earlier ones) as fallback registry, registry, resolver.

I do not currently handle ordering of different module types in this PR. So if you had both a pod style and non-pod style module with the same name whichever was last in requirejs.entries would win out. I could add a queueing system that works like moduleNameLookupPatterns here if you would like.

[stefanpenner]

I could add a queueing system that works like moduleNameLookupPatterns here if you would like.

I would be willing to forgo this if you can tell me why its not important. It seems important?

[rwjblue]

It seems important?

It is not important for the current need. We currently only need to know the names of the types in the system, and we do absolutely nothing with the factory or other information returned. All we need to know is whether a specific name exists (nothing more).

My needs for RFC#58 would be satisfied if eachForType simply returned a POJO like:

{
  'helper:t': true,
  'helper:blah': true
}

---

We could go with a simple true value (it is the minimum requirement ATM), we can always "upgrade" it to an object if we need to, and any usage of it (like if (returnValueFromEachForType['helper:t']) { }) would still work if/when we need to flesh it out as an object with more values (since that would also be truthy)...

[stefanpenner]

It is not important for the current need. We currently only need to know the names of the types in the system, and we do absolutely nothing with the factory or other information returned. All we need to know is whether a specific name exists (nothing more).

Ah this was the missing piece of information. Thank you! Priority/dedupe concern alleviated.

My needs for RFC#58 would be satisfied if eachForType simply returned a POJO like:

then this method name is likely misleading. As the result isn't an iterable rather an index/set of existence.

some ideas:

existsByType(type) === {
  'type:name: true
}

presentByType(type) === {
  'type:name;: true
}

[rwjblue]

Renamed to knownForType, and went with simple true value dictionary.

[stefanpenner]

sg

[mixonic]

lgtm :-D

[stefanpenner]

thanks for going on this journey with me

[teddyzeenny]

should module be renamed to defaultExport here?

[rwjblue]

Only if we would prefer to avoid an error 😜