-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
bug: Ionic / Angular Universal Prerender bugs on slides #21138
Comments
@liamdebeasi Do you guys have a customised express engine implementation or is the ionic ssr using the universal express engine purely without any changes ? I can't seem to find an example. |
This is an old project of mine using a few modifications to mock instances such as win and document, which fixes said issues, it's on top of the regular ngexpress. https://gist.github.com/NikolaPeevski/5a016d9edb46ec92ad99803fcf9f4558 |
Additionally there are some issues with inline css bootstrapping if one decides to use a crawler to generate facebook sharable thumbnails for example. I can provide an example that fixes that as well, it's a bit more clusterfunky tho. |
@NikolaPeevski This is not serving it and applying domino. This is prerender the app completely. See the github repo I added. |
@rgolea You can still mock the browser when pre-rendering, you need a prerender script similar to server.ts |
@NikolaPeevski would you be able to clone the provided git repo and show me a way to do that? |
@rgolea Check this out, https://github.com/Angular-RU/angular-universal-starter/blob/master/prerender.ts, I can also clone it and show you an example later. When you generate pages if I remember correctly it's the same principle as server side rendering them, as in you still have some javascript chunks that will be rendered in the browser, you can isolate them with the platform service. But in your case mocking it with a custom prerenderer is enough since you only care about the window. |
@NikolaPeevski So, the problem is not how to implement SSR. The problem is Ionic's incompatibility with SSR.
After this I get the
Afterwards, I still have issues with
After a couple of hours of hitting every dead end possible, I still get:
If you have any new ideas on how to get it to work when I run |
@liamdebeasi @NikolaPeevski Please check out this PR#4 and, if you can, please encourage the team to merge it so this issue can be solved. |
Waiting for this merge as well |
In the meanwhile, anyone who's interested, can just do the following:
This is theoretical, I didn't test it. If someone does, please leave a reply. |
Hmm so it sounds like this is not a bug in Ionic but rather a bug in that ssr-window dependency. Am I understanding your comment (#21138 (comment)) correctly, @rgolea? Looks like the developer is working with you on getting the fix merged in. When that fix has been merged and released I will update Ionic to pull in the latest version of swiper/ssr-window. |
Yes! Thank you @liamdebeasi. Does Ionic use |
Yeah, I think you are correct -- ssr-window is a dependency of swiper: https://github.com/nolimits4web/swiper/blob/master/package.json#L77. Not sure what the dev's release process is, but we'll get the fix into Ionic one way or another. |
Hey guys! The PR has merged with some extra features. If you want to, please try over the weekend the following: |
Quick update! v.2.0.0-beta.8 works. Confirmed! |
The author released 2.0.0 stable 2 days ago. Just need to wait for Swiper to get updated with the In the meantime, I created a dev build of Ionic with a custom build of Swiper. This build has Ionic Framework v5.1.1 and Swiper 5.3.8 + ssr-window 2.0.0. Here is a dev build if anyone would like to test: I tested this using the repo in the original post, and it seems like this fixes the issue. |
@liamdebeasi you're the best! Thank you! I will keep you posted and tell you when swiper gets the latest update. |
@liamdebeasi swiper has updated to the latest version. I will test tomorrow and confirm it works. |
Looks like the dependency in the |
@liamdebeasi I will check with him. |
Thanks for the issue. The Swiper dev released Swiper 5.4.1 with the dependency update, and I have updated Ionic with this via #21350. This fix will be available in an upcoming release of Ionic Framework. |
Also a big "thank you" to @rgolea for helping with testing and getting this issue fixed! 🎉 |
Thank you @liamdebeasi. Any time! Hope it helps others as well! |
Also thank you 👍 ! |
I confirm that this fixes the issue. In addition, ion-menu problem on angular universal is also fixed.. Now waiting this version to be released... Thanks. |
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. |
Bug Report
Ionic version:
[x] 5.x
Current behavior:
The SSR prerender does not render properly. There is a problem when importing
slider.js
. It seems that the screen object is not defined.Expected behavior:
It should allow to prerender the html files for the templates that contain
ion-select
to be prerendered.Steps to reproduce:
It happens when running
npm run prerender
directly from the@nguniversal/express-engine
schematics and the template has anion-slides
.Related code:
A sample application via GitHub
https://github.com/rgolea/ionic-ssr-errors
Other information:
This is a continuation of the #21063 since the original issue was fixed.
Ionic info:
Errors:
The error seems to come from here:
https://github.com/ionic-team/ionic/blob/6a167172ffa4802ff68b5ca0690586438f5ed744/core/src/components/slides/swiper/swiper.bundle.js#L2586
The thing is that it seems to be defined:
The text was updated successfully, but these errors were encountered: