fix(@angular-devkit/build-angular): bring back style tags in browser builder

[jkrems]

Adds tests for extracting component styles from <style> tags inside of the template. The application builder didn't have this regression but I still added a test there as well to ensure we don't accidentally regress on that side in the future.

So... what happened here? Basically, readResource returning a string and not a Promise was triggering what I suspect was unintended behavior:

1. In AdapterResourceLoader.preload, a string result from readResource leads to the function returning undefined instead of Promise<undefined>.
2. In preloadAndParseTemplate, an undefined return value of preload results in an immediate return Promise.resolve(null) while a Promise.resolve(undefined) actually populates the template cache and returns the template value.
3. Without a template value, the ComponentDecoratorHandler.preanalyze method will always return templateStyles: [] - completely ignoring what might have actually been in the template. Which is kind of fair - it never got to see the value of template.

Somewhere along the lines, something disagrees about the meaning of undefined and/or if there is a semantic difference between undefined and Promise<undefined>. I'm pretty sure what we want here is the behavior of Promise<undefined> where it actually takes the template metadata.

[dgp1130]

Thanks for looking into this! Seems like a tricky issue.

I feel like this comment is particularly telling: https://github.com/angular/angular/blob/92f30a749d676a290f5e173760ca29f0ff85ba8c/packages/compiler-cli/src/ngtsc/annotations/component/src/resources.ts#L435-L436

"If the preload worked" seems to mean "If it returned a Promise", which seems wrong to me? The timing of the function shouldn't indicate whether or not it was successful. I wonder if we should change that file to parse the template regardless of whether readResource returned undefined or Promise<undefined>. WDYT?

I'm not sure I'm seeing the "preload didn't work" cause in readResource, but if necessary, maybe we need to return whether or not it was successful independently of whether it returned a undefined or Promise<undefined>?

If this is sufficient to get the test to pass, I'm probably ok to merge it with Promise.resolve(content) as the fix just to avoid blocking v19, but we should definitely get a look from Charles on Monday before we merge it.

[jkrems]

If nothing else, I believe this kind of "undefined and Promise<undefined> have two different meanings" design deserves a closer look. I don't think there's a reason for this kind of API without much of a hint about the semantics of the values. The change here is meant to be least disruptive to reduce risk for v19. I hope we can fix this properly in the coming weeks even if we land this PR as-is.

[jkrems]

The changes were merged into the following branches: main, 19.0.x

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