Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jnizet opened this issue Nov 13, 2023 · 7 comments · Fixed by #27529

Comments

@jnizet
Copy link
Contributor

jnizet commented Nov 13, 2023

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.

@ngbot ngbot bot modified the milestone: needsTriage Nov 13, 2023
@jessicajaniuk jessicajaniuk transferred this issue from angular/angular Nov 13, 2023
@dgp1130
Copy link
Collaborator

dgp1130 commented Nov 14, 2023

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
Copy link
Contributor Author

jnizet commented Nov 14, 2023

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
Copy link
Collaborator

This is a Domino/platform-server issue.

@aarhusgregersen
Copy link

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
Copy link

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

@alan-agius4 alan-agius4 changed the title SSR: scripts not added to generated page if self-closing tag used for root component in index.html SSR/SSG: scripts not added to generated page if self-closing tag used for root component in index.html Apr 24, 2024
@ngbot ngbot bot removed this from the needsTriage milestone Apr 25, 2024
@alan-agius4 alan-agius4 transferred this issue from angular/angular Apr 25, 2024
@alan-agius4 alan-agius4 self-assigned this Apr 25, 2024
@alan-agius4 alan-agius4 added type: bug/fix freq1: low Only reported by a handful of users who observe it rarely severity3: broken labels Apr 25, 2024
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Apr 25, 2024
…ndex HTML

When an invalid self-closing HTML element is used in the index.html file, the build proceeds without raising an error, potentially leading to runtime issues.

Now, when an invalid self-closing element is encountered in the index.html file, the build process issues an error, providing developers with immediate feedback to correct the issue.

Closes: angular#27528
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Apr 25, 2024
…ndex HTML

When an invalid self-closing HTML element is used in the index.html file, the build proceeds without raising an error, potentially leading to runtime issues.

Now, when an invalid self-closing element is encountered in the index.html file, the build process issues an error, providing developers with immediate feedback to correct the issue.

Closes: angular#27528
alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Apr 25, 2024
…ndex HTML

When an invalid self-closing HTML element is used in the index.html file, the build proceeds without raising an error, potentially leading to runtime issues.

Now, when an invalid self-closing element is encountered in the index.html file, the build process issues an error, providing developers with immediate feedback to correct the issue.

Closes: angular#27528
@penfold
Copy link

penfold commented Apr 25, 2024

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.

alan-agius4 added a commit to alan-agius4/angular-cli that referenced this issue Apr 25, 2024
…ndex HTML

When an invalid self-closing HTML element is used in the index.html file, the build proceeds without raising an error, potentially leading to runtime issues.

Now, when an invalid self-closing element is encountered in the index.html file, the build process issues an error, providing developers with immediate feedback to correct the issue.

Closes: angular#27528
alan-agius4 added a commit that referenced this issue Apr 25, 2024
…ndex HTML

When an invalid self-closing HTML element is used in the index.html file, the build proceeds without raising an error, potentially leading to runtime issues.

Now, when an invalid self-closing element is encountered in the index.html file, the build process issues an error, providing developers with immediate feedback to correct the issue.

Closes: #27528
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants