ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.02k stars 13.51k forks source link

bug: angular, moving elements inside of ion-router-outlet causes dom tree mismatch during hydration #28534

Closed bashoogzaad closed 9 months ago

bashoogzaad commented 11 months ago

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

When starting a new project with ionic start with the option Angular Standalone, then @angular/ssr is not working correctly. When adding @angular/ssr, the Angular CLI is adding files, such as app.config.server.ts. This file is referring to app.config.ts. This file is however not present, so you have to create is. Not a big deal.

However, after npm run dev:serve, the application is running, but gets an error:

main.ts:12 ERROR Error: NG0501: During hydration Angular expected more sibling nodes to be present.

Actual DOM is:

<ion-app>
  …
  <ion-router-outlet>…</ion-router-outlet>
  <!-- container -->  <-- AT THIS LOCATION
</ion-app>

To fix this problem:
  * check corresponding component for hydration-related issues
  * check to see if your template has valid HTML structure
  * or skip hydration by adding the 'ngSkipHydration' attribute to its host node in a template

 Find more at https://angular.io/errors/NG0501
    at validateSiblingNodeExists (core.mjs:18669:15)
    at siblingAfter (core.mjs:19187:22)
    at locateDehydratedViewsInContainer (core.mjs:19395:32)
    at populateDehydratedViewsInLContainerImpl (core.mjs:20174:44)
    at locateOrCreateAnchorNode (core.mjs:20189:10)
    at createContainerRef (core.mjs:20086:5)
    at injectViewContainerRef (core.mjs:19846:12)
    at core.mjs:4188:29
    at runInInjectorProfilerContext (core.mjs:867:9)
    at lookupTokenUsingNodeInjector (core.mjs:4187:17)

This is just a starter project with 'list' prefill.

Expected Behavior

The application should run correctly, and must be able to hydrate the components.

Steps to Reproduce

  1. ionic start -> Angular Standalone -> list prefill
  2. ng add @angular/ssr
  3. Create app.config.ts based on main.ts
  4. npm run dev:ssr

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 7.1.5 (/usr/local/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 7.5.4 @angular-devkit/build-angular : 17.0.0 @angular-devkit/schematics : 17.0.0 @angular/cli : 17.0.0 @ionic/angular-toolkit : 9.0.0

Capacitor:

Capacitor CLI : 5.5.1 @capacitor/android : not installed @capacitor/core : 5.5.1 @capacitor/ios : not installed

Utility:

cordova-res : not installed globally native-run (update available: 2.0.0) : 1.7.4

System:

NodeJS : v18.18.2 (/usr/local/Cellar/node@18/18.18.2/bin/node) npm : 9.8.1 OS : macOS Unknown

Additional Information

No response

ionitron-bot[bot] commented 11 months ago

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

liamdebeasi commented 11 months ago

Can you provide a reproduction we can run? SSR issues can be a bit tricky, so having a full sample will save us a lot of time.

bashoogzaad commented 11 months ago

Hi @liamdebeasi , thanks for your quick response! I have created a repo here: https://github.com/bashoogzaad/ionic-ssr-test

liamdebeasi commented 11 months ago

Does the issue persist if you use @ionic/angular-universal? https://ionic.io/blog/ssr-with-angular-universal-and-ionic

bashoogzaad commented 11 months ago

I am not able to use that, that one is based on ngModules and I am using Angular Standalone (as the preferred solution nowadays). And Angular SSR should work out of the box in v17 (maybe this is related: https://forum.ionicframework.com/t/angular-universal-update/213522/2)

liamdebeasi commented 11 months ago

You should be able to use NgModules in a standalone application still: https://angular.io/guide/standalone-components#using-existing-ngmodules-in-a-standalone-component. The module would be passed to the imports array in your app component.

Also, I'm not able to reproduce the issue in your sample repo:

Error: src/app/app.config.server.ts:3:27 - error TS2307: Cannot find module './app.config' or its corresponding type declarations.

3 import { appConfig } from './app.config';

Can you make sure there's an app.config file in the repo?

bashoogzaad commented 11 months ago

Excuse me, I have added it now!

liamdebeasi commented 11 months ago

Thanks! So the problem appears to be that we are moving children inside of ion-router-outlet. By default, the current view's component is mounted as a sibling of ion-router-outlet. However, the component needs to be a child of ion-router-outlet so things like page transitions and swipe to go back can work properly.

To work around this, we move the element inside of ion-router-outlet: https://github.com/ionic-team/ionic-framework/blob/9d57758e3ec1095c6eb951d2640b95084b711bbc/packages/angular/common/src/directives/navigation/stack-controller.ts#L278

This causes a DOM tree mismatch during hydration because Angular was expecting the view's component to be a sibling of ion-router-outlet. According to https://angular.io/guide/hydration#third-party-libraries-with-dom-manipulation, you can avoid this by setting ngSkipHydration on the component that renders ion-router-outlet.

However, this would mean setting ngSkipHydration on the root element which defeats the purpose of having hydration in the first place. To fix this, we likely need to update our implementation to ensure that view components are mounted in the correct place the first time so we don't need to move anything around in the DOM.

liamdebeasi commented 11 months ago

Also wanted to set appropriate expectations: @angular/ssr does not support Web Components at the moment, so even if we fix this issue (which we should as there are positive performance implications for apps if we address this) there will still be other issues. For example, Stencil renders comments inside of scoped components for style encapsulation. These comments cause @angular/ssr to break. A good example of this is the ion-header component.

Unfortunately, this is not something we can address as the Angular team needs to add support for Web Components to the @angular/ssr package. If this is something import for your use case, I recommend providing feedback to the Angular team on https://github.com/angular/angular/issues.

stevebrowndotco commented 10 months ago

Relevant link: https://github.com/angular/angular/issues/52275

stevebrowndotco commented 10 months ago

@liamdebeasi might be worth noting that the Lit framework team, which also uses web components, is claiming that they are working on SSR support now, according to here. https://lit.dev/docs/ssr/overview/ Maybe the Ionic Team could consider a similar approach?

liamdebeasi commented 10 months ago

We'd probably still advocate for getting Web Component support in @angular/ssr. Even if Stencil had a Stencil-specific SSR solution, the Angular team (and React and Vue teams) would likely need to add Stencil-specific integrations to make that SSR solution compatible with each JS Framework. It's certainly possible, but it would be a sizable effort to build and maintain all of these different integrations.

eideard-hm commented 10 months ago

Does this mean that server side rendering is not yet possible?

liamdebeasi commented 9 months ago

Thanks for the issue. This has been resolved via https://github.com/ionic-team/ionic-framework/pull/28544, and a fix will be available in an upcoming release of Ionic Framework.

We chose to ship this in a major release of Ionic Framework to account for any unexpected behavior changes. There should be no behavior changes, but how views are mounted are important to the functionality of page transitions and swipe to go back, so we'd like a public testing period.


Does this mean that server side rendering is not yet possible?

You can use the @ionic/angular-server package with Angular Universal for some SSR support. However, Angular's @angular/ssr package does not support Web Components yet and cannot be used with Ionic.

ionitron-bot[bot] commented 8 months ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.