Addons can provide their resolver module configuration via resolverConfig()

[ppcano]

The PR gives the ability to addons to provide their own resolver configuration via a new addon hook resolverConfig as per RFC-262.

This requirement was initially created at the QUEST Module Unification - Final Cut and supercedes ember-cli/ember-cli#8419

Ember addons need an API that allows them to alter the module unification config

An addon can provide its resolve module configuration for MU apps like:

// my-addon/index.js
module.exports = {
  name: require('./package').name,
  resolverConfig() {
    return {
      collections: {session: { definitiveCollection: 'session' }},
      types: {translation: { definitiveCollection: 'main' }}
    };
  }
};

For reference, the MU default config shows an example with the different options.

[NullVoxPopuli]

Can an addon overwrite the default MU module config? Atm mergeAddonsConfig will throw an exception.

I think addons should not have the power to change any part of the config, only add to it.
Changing / mutating could lead to some weird behavior, imo

If two addons overwrite the same type or collection

if the configuration ends up being the same would it matter? idk how hard that logic would be to implement

[rwjblue]

• This seems to only gather resolver config from top level addons at the moment, we should decide if that is what we want or not (I think we should probably recurse and process nested addons as well).
• The new code should be feature flagged (in the same way that the MU tree work is done today) to ensure no impact when not using MU
• Its not clear to me why this is implemented in terms of app runtime instead of build time only.

[rwjblue]

I do not understand why this is included in addon/ and not done completely in index.js, what am I missing?

[ppcano]

@rwjblue What is your proposal in this case? Let's see if I can understand your suggestion.

In this proposal, the resolver looks like:

     import Resolver from 'ember-resolver/resolvers/fallback';
     import buildResolverConfig from 'ember-resolver/ember-config';
     import config from '../config/environment';
     import addonsConfig from 'ember-resolver/addons-config';
     import mergeAddonsConfig from 'ember-resolver/merge-addons-config';

     let moduleConfig = buildResolverConfig(config.modulePrefix);
     mergeAddonsConfig(moduleConfig, addonsConfig);

     export default Resolver.extend({
       config: moduleConfig
     });

The only benefit is that users can modify the addonsConfig to skip the addon collision exception.

Are you suggesting to perform the addon config merge within the buildResolverConfig function and continue using the current MU resolver.js like:

     import Resolver from 'ember-resolver/resolvers/fallback';
     import buildResolverConfig from 'ember-resolver/ember-config';
     import config from '../config/environment';
     

     let moduleConfig = buildResolverConfig(config.modulePrefix);

     export default Resolver.extend({
       config: moduleConfig
     });

or something else?

[rwjblue]

I'm suggesting to do the final merging in node land and using broccoli-file-creator to write the final config to ember-resolver/ember-config directly.

[ppcano]

@rwjblue

I think autogenerating the whole ember-resolver/ember-config is not ideal, because this is a function that needs the config.modulePrefix.

I have changed my implementation to execute mergeAddonsConfig within the ember-resolver/ember-config which has been refactored:

import addonsConfig from 'ember-resolver/addons-config';
import moduleConfig from 'ember-resolver/module-config';
import mergeAddonsConfig from 'ember-resolver/utils/merge-addons-config';

export default function generateConfig(name) {

  let config = {
    app: {
      name,
      rootName: name
    },
  };

  mergeAddonsConfig(moduleConfig, addonsConfig);

  return Object.assign(config, moduleConfig);
}

This setup allows using the same default src/resolver.js as today.

     import Resolver from 'ember-resolver/resolvers/fallback';
     import buildResolverConfig from 'ember-resolver/ember-config';
     import config from '../config/environment';
     
     let moduleConfig = buildResolverConfig(config.modulePrefix);

     export default Resolver.extend({
       config: moduleConfig
     });

Done at 4d29950

[ppcano]

@rwjblue

This seems to only gather resolver config from top level addons at the moment, we should decide if that is what we want or not (I think we should probably recurse and process nested addons as well).

I didn't know about that. I am gonna test it. If this is the case, I have to figure out how to recurse the nested addons at the ember-resolver or ember-cli project.

The new code should be feature flagged (in the same way that the MU tree work is done today) to ensure no impact when not using MU.

Yes, it is. this._moduleUnificationTrees is only executed if isModuleUnification is enabled.

Its not clear to me why this is implemented in terms of app runtime instead of build time only.

I don't know how it could be the possible implementation, I wrote you about it at #348 (comment).

[ppcano]

@rwjblue

we should probably recurse and process nested addons as well

Done by 3762945

Its not clear to me why this is implemented in terms of app runtime instead of build time only

Replied at #348 (comment)

[rtablada]

Something I'm worried about is documenting the difference between registering collections and registering types.

Before the hard assertion on types/collections many add ons (and some applications) injected or added things to the app container. While the resolver is a rather advanced topic I'm not sure we are giving enough documentation to users on how to resolve errors in their applications or contribute fixes to add ons.

[rtablada]

To be a bit more straightforward about it. We're currently running into issues trying to use the new resolver in our application and are getting errors for both ember-simple-auth and ember-can right now reading this PR I wouldn't know if those errors would be resolved with registering a collection, a type, or both.

Right now I'd have to stab in the dark until things started working. While possible and not too much of an issue for bleeding edge, this is not an experience we'd want for normal app or add-on developers.

[ppcano]

@rtablada Totally agree with your concerns.

While we work on the API design, RFC and documentation, rfc013#Module Naming and Organization and the MU module configuration are good resources to learn how to customize the app module configuration.

Before an addon API is available and addons that need their own modules implement support for the MU layout, you will have to change src/resolver.js to support using the addon collection and addon types like emberjs/ember.js#17234 (comment).

Note: types and collection for addons are registered like the registration of your own app types and collections.

reading this PR I wouldn't know if those errors would be resolved with registering a collection, a type, or both.

The proposal is that app developers don't have to setup the addon module configuration; addons must configure their own collection and types and app developers just follow the addon instructions as today.

We're currently running into issues trying to use the new resolver in our application and are getting errors for both ember-simple-auth and ember-can right now

I don't know the particularities of these addons, but please let us know if you find something that has not been considered in this PR.

[rwjblue]

Closing now that we have removed module unification support code.

Thanks for all of your hard work here @ppcano, sorry we weren't able to get it landed in a timely fashion.