angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.76k stars 11.98k forks source link

SSR/SSG: scripts not added to generated page if self-closing tag used for root component in index.html #27528

Closed jnizet closed 5 months ago

jnizet commented 11 months ago

Which @angular/* package(s) are the source of the bug?

platform-server

Is this a regression?

No

Description

When using <app-root /> in index.html in an SSR-enabled app, with the application builder, instead of <app-root></app-root>, then the scripts are not added to the generated page on the server.

When SSR is disabled, the self-closing tag works fine.

Even though one might argue that index.html is not a component template and that <app-root /> is not valid HTML, since it works without SSR but fails with it, the problem is not trivial to identify.

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/stackblitz-starters-9sgq9l?file=ssrbug%2Fsrc%2Findex.html

Please provide the exception or error you saw

Execute the following commands:

cd ssrbug
npm install
npm run start

See that the button increments the counter as expected.

Edit the index.html file and replace <app-root></app-root> with <app-root />. Run npm run start again. See that the button click doesn't work anymore. The JS scripts are not present in the generated HTML page anymore.

Edit the angular .json file and remove those properties, to disable SSR:

            "server": "src/main.server.ts",
            "prerender": true,
            "ssr": {
              "entry": "server.ts"
            }

Run npm run start again. See that the button works fine even with a self-closing tag when SSR is disabled.


### Please provide the environment you discovered this bug in (run `ng version`)

```true
Angular CLI: 17.0.0
Node: 18.18.0
Package Manager: npm 9.4.2
OS: linux x64

Angular: 17.0.2
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, platform-server
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1700.0
@angular-devkit/build-angular   17.0.0
@angular-devkit/core            17.0.0
@angular-devkit/schematics      17.0.0
@angular/cli                    17.0.0
@angular/ssr                    17.0.0
@schematics/angular             17.0.0
rxjs                            7.8.1
typescript                      5.2.2
zone.js                         0.14.2

Anything else?

This bug has been discovered by helping someone on the Angular discord. So it happened in a real-life scenario.

dgp1130 commented 11 months ago

Even though one might argue that index.html is not a component template and that <app-root /> is not valid HTML, since it works without SSR but fails with it, the problem is not trivial to identify.

I would exactly make this argument. We expect index.html to be valid HTML, Angular templates are a different syntax where we can get away with /> in a way which doesn't apply to HTML more generally. The fact that <app-root /> works for CSR builds is probably unintentional in the first place.

I can see an argument that CSR and SSR should probably align their behavior, but I'd probably lean towards not working in both cases.

That said, we could probably do better with an error message here. The exact semantics of /> is not well understood by many web devs (obligatory blog post by Jake Archibald), and I could definitely see some users "simplifying" their index.html and being confused by the result. We could consider running an HTML validator on the index.html page to make sure it stays conformant, though I'm not completely convinced that's worth the impact on build times and extra dependencies. Something we should think about at least.

jnizet commented 11 months ago

Agreed. That was the point: an error message that would make things clearer. Although I would also argue that, since Angular decided to relax the rules on HTML in templates, and since the index.html is processed by the Angular SSR build to transform it into something different (the body o <app-root> is filled, attributes are added, etc.), the same relaxed rules could apply.

alan-agius4 commented 11 months ago

This is a Domino/platform-server issue.

aarhusgregersen commented 6 months ago

Being someone myself who recently spent a solid days worth of work narrowing down an issue to this exact issue, an error message would have been helpful.

Not exactly sure how that error message should be presented, but wondering if leaving just an HTML comment in the index.html above the <app-root>saying not to self-close it, would be enough.

g-cheishvili commented 6 months ago

Spent good chunk of the day understanding what the hell was going wrong with my application. So the app works fine if you do not use SSR, but if you use, you lose all the events! Some sort of message would be very helpful, but would be way better if this was supported, and the point is not only about the self closing tags, but also the bootstrap component having attribute selector(the case that I had). I, for example, do not like extra tag in HTML. So having a body, with some attribute is not a wrong HTML per se, but still got caught up in the mess

penfold commented 5 months ago

We came across this issue when we were refactoring our codebase to use self-closing tags for all NG components. This mindset led us to set the root component in the index.html to be self-closing along with all the other NG components in the project.

Only finding this issue when we rebuilt and deployed to a hosted environment meant that we had a large feature PR of changes to pick through.

A build error is needed to highlight the invalid index.html otherwise it is very time-consuming to look for an unknown cause of the failure.

angular-automatic-lock-bot[bot] commented 4 months ago

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.