feat(@ngtools/webpack): allow .svg files as templates

[oocx]

With directTemplateLoading enabled, components
can now use .svg files as templates. For AOT builds,
the Angular compiler host now reads .svg files
directly when reading component templates.
For JIT builds, replaceResources creates a require call
that directly uses raw-loader instead of using the
loader provided by the current webpack configuration.

Closes #10567

[alan-agius4]

This might be a breaking change, if for a reason at the moment you have a file named .component.svg It’s behaviour will change.

[alan-agius4]

See above comments.

I still think that this might be a breaking change in some cases.

[oocx]

Should I just add a breaking change note to the commit message? I can't think of a way to implement this that's not a potentially breaking change.

[alan-agius4]

I think we should try to use the issuer. See: #10567 (comment)

[oocx]

This could also be a breaking change if you already import .svg from .js files.

[filipesilva]

I'm not too keen on the .component.svg convention. We've tried to stay away from having to name your files in a certain way to use them because it's a somewhat brittle convention, and it's hard to inform users of it as well. I will bring it up with the team to see what everyone thinks.

[oocx]

I've just spent several hours debugging webpack. For some reason I don't understand yet, webpack does not always provide an issuer.

When you look at webpack\lib\RuleSet.js, there is a method _run that evaluates rules. The first parameter to run is an object "data", which has the object against which the rule should be evaluated. Here, data.issuer is not always set, in many cases (especially in my experiments with using a .svg as template), issuer is set to "".

I currently don't see a way to use issuer here without hours of reverse engineering webpack.

I'll ask for help in the webpack gitter. However, I already spent about 8 hours today for a simple change that is not even 5 lines of configuration (maybe 15 minutes for the actual change, a few hours to figure out how to unit test it, then again several hours to work on feedback). If I don't find a solution soon, the problem is just not worth investing more time.

[oocx]

@alan-agius4 as requested, I've added a test that uses a component and aot. Is this what you had in mind, or am I still missing something?

I'm still switching loaders based on the .component.svg suffix, as the idea to use the issuer does not seem to work.

[alan-agius4]

Thanks @oocx, as @filipesilva mentioned we’ll discuss this with the rest of the team in our next meeting.

[alan-agius4]

Can you kindly fix the commit message as the feature is bound to @ngtools/webpack ?

[alan-agius4]

Hi @oocx, are you willing to continue on this and address the above comments?

Thanks.

[oocx]

@alan-agius4 yes, I can probably continue working on it next weekend

[oocx]

I hope I understood and implemented all the requested changes correctly this time.

[alan-agius4]

@oocx, ci is red.

[oocx]

@alan-agius4 Thanks. I'm sorry, I don't know how I could miss that. I'll fix that next weekend.

[oocx]

"ci/angular: size — cli/new-production/test-project/main.js increased by 24.00KB."

How can I troubleshoot that? Is there a way to get details for that check? (for exmple, get a diff of the increased output vs. thee baseline)? I don't see a size increase with my local build (for "ng build --prod", but I don't even know which build settings this check tests).

[clydin]

For the size check, try rebasing against the latest upstream master.

[oocx]

@clydin thanks, rebasing helped.

[filipesilva]

LGTM, great work!

I left a question but it's non blocking really.

[filipesilva]

@oocx this is now merged and should be part of 8.0. Thanks for all the work you've done on it!

[oocx]

Thanks everyone for your feedback! You have been very helpful and provided valuable feedback, so this was a great experience for me as new contributor.

[alan-agius4]

@oocx, thanks for sharing your experience. Looking forward to have more PRs from you in the near future :)

Thanks again for your contribution.

[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.}