feat(ivy): implement listing lazy routes for specific entry point in ngtsc - #28542

Oopsie

I know, I know 😃 (This is not really ready for review - sorry for the misleading label 😞)

ngtsc complained about non-existent component.

As well it should!

These toString() methods are really helpful when debugging and in errors. I accept that there is no real need for the abstract version since all Objects have this method defined already.

Happy to bring that back then 😃

I don't believe you need the as 'ngtsc' here.

Without it, the type of the object ends up as {enabledIvy: false | string}, which TS complains is not assignable to 'ngtsc' | boolean | ....

Really? That is not what I see. {enableIvy: ivyEnabled && 'ngtsc'} appears to be enough.

Oh, you are right. Originally, I had it as a separate variable:

const overrideOptions = {enableIvy: ivyEnabled && 'ngtsc' as 'ngtsc'};
const options = testSupport.createCompilerOptions(overrideOptions);

In that case, TS complains without the as 'ngtsc', but when using the object literal directly, it is not necessary 🎉
Good catch!

Should this fixmeIvy be removed if this commit is fixing something to work with ivy?

I removed it in a subsequent commit, because this commit (although enough to make routes work in cli projects) was not enough to fix the test.

I think this error message should have a bit more context. Perhaps:

Resolution of relative paths to lazy routes is not supported.

My initial intention was to keep this close to the old AotCompilerHost implementation, but I don't see any reason to 😁
I'll add more info to the error.

I don't find defining such functions via fat arrows very readable. Although I accept that the terrible automated formatting is partly to blame.

This is what it would look like as a normal function:

function lazyRouteMatching(
    route: string, fromModulePath: RegExp, fromModuleName: string, toModulePath: RegExp,
    toModuleName: string) {
  return {
    route: route, module: jasmine.objectContaining(
                      {name: fromModuleName, filePath: jasmine.stringMatching(fromModulePath)}),
        referencedModule: jasmine.objectContaining(
            {name: toModuleName, filePath: jasmine.stringMatching(toModulePath)}),
  }
  as unknown as LazyRoute;
}

Feel free to turn off the clang-formatter and do a better job of making the body readable :)

Given that this is testing "all" routes, should we not have more than one route in the test?

Good point. I was just refactoring what was there before.

Do you need to have components for the compilation to work? If not is this unnecessary noise in this test?

This is to ensure that having other things than NgModules in the exports does not throw the RouteAnalyzer off.
I can split it out into a separate test.

Nice comment, thanks!

Should ignore and not throw? Is that the specified behaviour? I guess this would throw later if you tried to navigate to that route?

Yeah, that's a good question. Not sure what it should do (but added a test for what it actually does right now).

I chose the current behavior (not to throw if it doesn't know about a lazy route's module), because that is somewhat similar to what would happen if listLazyRoutes() was called without an entry point: It would list all routes found in the know modules, but it would not complain if a lazy route points to an unknown module.

I can be convinced otherwise, though 😃

As well it should!

I'm actually planning on doing that soon anyway, for all of the tests in it. Stay tuned!

Feel free to turn off the clang-formatter and do a better job of making the body readable :)