[Module Unification] Routes can only be nested two levels deep

[buschtoens]

Defining routes nested more than two levels deep or even just providing a callback does not work MU applications and triggers the error below.

requirejs.js:74 Uncaught TypeError: Cannot read property 'defaultType' of undefined
    at RequireJSRegistry._checkDefaultType (requirejs.js:74)
    at RequireJSRegistry._detectModule (requirejs.js:63)
    at RequireJSRegistry.has (requirejs.js:85)
    at Resolver._serializeAndVerify (resolver.js:118)
    at Resolver.identify (resolver.js:94)
    at Resolver.resolve (resolver.js:104)
    at Class.resolve (index.js:149)
    at Class.resolve (index.js:15)
    at Class.superWrapper [as resolve] (utils.js:308)
    at _resolve (container.js:807)

Exemplary failing router:

Router.map(function() {
  // this fails
  this.route('foo', function() {
    this.route('bar', function() {
      // You don't even need to register a third-level route. Just providing the
      // callback triggers the error, likely because of the implicit `index` route.
      // this.route('qux');
    });
  });

  // this works just fine
  this.route('one', function() {
    this.route('two');
  });
});

Reproduction repo: https://github.com/buschtoens/mu-nested-routes

[buschtoens]

The problem seems to be somewhere in glimmer-resolver/resolver.ts#identify. Routes generate non-sensical specifiers with an IMO incorrect rootName property:

{
  type: "route",
  name: "index",
  namespace: undefined,
  collection: "routes",
  rootName: "foo/bar"
}

I think it should rather look like this:

{
  type: "route",
  name: "foo/bar/index",
  collection: "routes",
  rootName: "mu-nested-routes"
}

Or maybe?

{
  type: "route",
  name: "index",
  namespace: "foo/bar",
  collection: "routes",
  rootName: "mu-nested-routes"
}

[buschtoens]

I assume the glimmer-wrapper in ember-resolver is the correct place to submit a failing test case to.

[NullVoxPopuli]

@buschtoens have you tried the fallback resolver?
the fallback resolver is currently what https://emberclear.io uses

[buschtoens]

@NullVoxPopuli I am already using the fallback resolver, as can be seen here. But that is actually a very good point to make. I'll give the glimmer-wrapper a try instead. Thanks a lot!

[buschtoens]

@NullVoxPopuli No luck here. Apparently fallback and glimmer-wrapper use the same buggy code paths.

fallback first tries the glimmer-wrapper resolver, and if that one cannot find a result, will fall back to the classic resolver:

https://github.com/ember-cli/ember-resolver/blob/7647d14d242b08d40305dfdbef2e028f1a19b4de/mu-trees/addon/resolvers/fallback/index.js#L6-L15

[NullVoxPopuli]

excellent! (one less thing to fix)

[buschtoens]

Ha! I just found out that this only happens, if the third-level Route or Controller are auto-generated. If the app actually provides a route.js and controller.js, there's no error. I've updated the reproduction repo to show this as well.

[buschtoens]

I think I've pinned down the issue. So in L90 s looks like this:

{
  type: "controller",
  name: "index",
  namespace: "foo/bar",
  collection: "routes",
  rootName: "dummy"
}

The resolver cannot find the Controller, as it is not explicitly defined and thus should be auto-generated. But before resorting to auto-generation, the resolver tries as to find a fitting addon first in L92-L100.

s then looks like this:

{
  type: "controller",
  name: "index",
  namespace: undefined,
  collection: "routes",
  rootName: "foo/bar"
}

This is then serialized as controller:/foo/bar/routes/index in _serializeAndVerify by serializeSpecifier. It is then deserialized in has as:

{
  type: "controller",
  name: "index",
  namespace: "routes",
  collection: "bar",
  rootName: "foo"
}

Notice how this differs from the prior specifier:

{
  type: "controller",
  name: "index",
-  namespace: undefined,
+  namespace: "routes",
-  collection: "routes",
+  collection: "bar",
-  rootName: "foo/bar"
+  rootName: "foo"
}

What ultimately causes the TypeError then is that that _checkDefaultType expects the specified collection to always exist:

  _checkDefaultType(specifier) {
    let {defaultType} = this._config.collections[specifier.collection];
    return defaultType && defaultType === specifier.type;
  }

A simple fix to make this safer would be:

  _checkDefaultType(specifier) {
    let { defaultType } = this._config.collections[specifier.collection] || {};
    return defaultType && defaultType === specifier.type;
  }

However this does not fix the inconsistent de-/serialization. But is this actually a problem we need to worry about? The input specifier already looks off to me.

In any case, I'll prep a PR for the above fix and test it.

[gossi]

This can be marked as closed 😃 Maybe cut a release for ember-resolver that contains the fix (it's already on master)

[locks]

Closing, thanks!