fix(@ngtools/webpack): allow # in paths

[dealloc]

Possible fix for #9100 by only splitting on # when it's followed by a valid classname

(This time commits are done with the correct email address)

[dealloc]

@filipesilva I don't really understand what / why it's failing? And locally my tests seem to fail even without me making any modifications at all (ie running tests on master fails as well)

[filipesilva]

I'm not sure which tests fail locally for you, but I think master is still passing CI. I restarted a couple of jobs on the latest master build and they still passed.

The error CI is showing isn't related to ng build itself failing, but rather the check that --aot is indeed working. This test is a good example:

Running "tests/basic/aot" (8 of 52 [0:4] (8/185))...
tests.basic.aot
  ==========================================================================================
  Running `ng "build" "--aot"`...
  CWD: /tmp/angular-cli-e2e-118010-4662-pdvc13.izy2xzuxr/test-project
    Date: 2018-01-10T08:17:00.010Z
    Hash: 78e7ff642af924d59053
    Time: 10454ms
    chunk {inline} inline.bundle.js (inline) 5.79 kB [entry] [rendered]
    chunk {main} main.bundle.js (main) 3.05 kB [initial] [rendered]
    chunk {polyfills} polyfills.bundle.js (polyfills) 202 kB [initial] [rendered]
    chunk {styles} styles.bundle.js (styles) 17.2 kB [initial] [rendered]
    chunk {vendor} vendor.bundle.js (vendor) 2.33 MB [initial] [rendered]
Last step took 13.64s...
Error: File "dist/main.bundle.js" did not contain "/platformBrowser.*bootstrapModuleFactory.*AppModuleNgFactory/"...

You can run just this e2e test by doing node tests/run_e2e.js tests/e2e/tests/basic/aot.

ng build --aot ran successfully but the bundle didn't have the bootstrapModuleFactory AOT code. I think that's because the entry module replacement failed.

I checked out your PR locally and tried to reproduce. Indeed after ng build --aot the bundle didn't contain bootstrapModuleFactory.

I added a console.log(path, className) to get entryModule() { and it showed the following:

D:/sandbox/master-project/src/app/app.module #AppModule  

This breaks because the # should be part of the class name.

I think your fix also would produce a bad path when there's a # in the folder names though, because you still only pick up the first part of the split.

So for /my/path#to/project/src/app/app.module.ts#AppModule it would say the path was /my/path.

I think the correct algorithm is to split using /(#[a-zA-Z]+)$/ and keep the previous logic. WDYT?

[dealloc]

Sorry for the (extreme) delay in replying, it's been hectic at work.
I ran the unit test locally (node tests/run_e2e.js tests/e2e/tests/basic/aot) and it produced no errors

You're right about the fact that I don't include the filename in the class (#AppModule instead of app.module.ts#AppModule), but I'm pretty confident that a # in other parts of the pathname will not break, a small reproduction (changed get to function and removed this references):

function entryModule(_entryModule) {
  if (!_entryModule) {
    return undefined;
  }
  const splitted = _entryModule.split(/(#[a-zA-Z]+)/);
  const path = splitted[0];
  let className = splitted[1] || 'default';
  if (splitted.length >= 3) {
    className = splitted[splitted.length - 2];
  }

  return { path, className };
}

const testPath1 = '/my/path#to/project/src/app/app.module.ts#AppModule';
const testPath2 = 'E:\C#46\trunk\DataRouting\Customer\AAA\TruckRegistration\Frontend';

console.info('Path 1:', entryModule(testPath1));
console.info('Path 2:', entryModule(testPath2));

Which yields the following results in repl.it:

But I should get the filename in the class output as well, I'll try to get on that as soon as possible (I have been assigned a different project for now so it might take a while I'm afraid).

EDIT: just noticed that your example with the path (/my/path#to/project/src/app/app.module.ts#AppModule) gives incorrect ouput, I tried your suggestion and changed it to the following code (repl.it here):

function entryModule(_entryModule) {
  if (!_entryModule) {
    return undefined;
  }
  const splitted = _entryModule.split(/(#[a-zA-Z]+)$/);
  const path = splitted[0];
  const className = splitted[1] || 'default';
  return { path, className };
}

const testPath1 = '/my/path#to/project/src/app/app.module.ts#AppModule';
const testPath2 = 'E:C#46\trunkDataRoutingCustomerAAATruckRegistrationFrontend';

console.info('Path 1:', entryModule(testPath1));
console.info('Path 2:', entryModule(testPath2));

Which yields the following output:

Your suggestion indeed fixed splitting the path halfway, but it seems we still need to get the filename in there.

[dinerotah]

@filipesilva The RegExp you suggested works fine I just had removed the leading # from className.
I had made a new PR #9236

[dealloc]

Guess I'll close this in favour of #9236 since it's got cleaner history and I see myself having the time to fix this today (or any time this week) with this crazy schedule.

Thanks @Taha-Di-Nero

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}