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

Add support for Angular Universal #230

Open
3 of 5 tasks
sinedied opened this issue Jan 29, 2018 · 17 comments
Open
3 of 5 tasks

Add support for Angular Universal #230

sinedied opened this issue Jan 29, 2018 · 17 comments
Assignees
Labels
Milestone

Comments

@sinedied
Copy link
Member

sinedied commented Jan 29, 2018

Now that Angular Universal has landed and is properly supported through Angular CLI, we should also support it and provide an out-of-the-box DX for SSR (Server-Side Rendering).

Tasks:

See also https://github.com/angular/universal-starter for reference

@sinedied sinedied added this to the @next milestone Jan 29, 2018
@creal73 creal73 self-assigned this Feb 22, 2018
@ToFab
Copy link

ToFab commented Apr 13, 2018

Any updates on this?

@wnabil
Copy link
Contributor

wnabil commented Jun 10, 2018

I followed the steps on https://angular.io/guide/universal,
the build is done fine and after clearing some errors just like 'document and window' variables i got an error Zone is not defined
Anyone else makes it work with ngx rocket ?

@sinedied
Copy link
Member Author

@creal73 is still working on this, Angular 6 may have changed some things regarding universal support so existing code needs to be adapted, stay tuned!

@creal73
Copy link
Collaborator

creal73 commented Aug 22, 2018

Work is in feature/universal branch. Need to update the code using "Gotchas" to prevent window, localStorage, sessionStorage running on server side

@gregor-srdic
Copy link

Any progress on this? Can you please consider adding pre-rendering scripts - something like "generate:prerender" in https://github.com/angular/universal-starter.

@creal73
Copy link
Collaborator

creal73 commented Jan 8, 2019

Work is still in progress and any help is welcome !

I'll have a look on your suggestion by the end of the week

@AnthonyNahas
Copy link

Supporting SSR would be really great! I was converting the generated project into angular universal app and I faced some further problems related to this topic --> localstorage has been used without to check whether the app is running on the client or server side ... (todo)

> cd dist && node prerender

Warning: Flex Layout loaded on the server without FlexLayoutServerModule
ERROR ReferenceError: localStorage is not defined
    at I18nService.set [as language] (/Users/anthonnahas/..../dist/prerender.js:114732:36)
    at I18nService../src/app/core/i18n.service.ts.I18nService.init (/Users/anthonnahas/.../dist/prerender.js:114712:23)
    at AppComponent../src/app/app.component.ts.AppComponent.ngOnInit

@sinedied
Copy link
Member Author

Yes, currently localStorage is used as part of the HttpCacheService, but maybe it's time to say goodbye to it and be more in favor of state management instead?

If would then simplify the work needed to make SSR working as a nice side-effect 😉

@AnthonyNahas
Copy link

@sinedied I agree ;)

@creal73
Copy link
Collaborator

creal73 commented Jan 30, 2019

Excellent idea !

@sinedied Do you want to use ngxs instead of ngrx ?

@sinedied
Copy link
Member Author

My preference would go to NGXS, but given the difference between the 2 it would not be too much work to add both and give the choice :)

@captaincaius
Copy link
Contributor

I just crossed this bridge myself finally and I think there are some design decisions that should be made for this. I wanted to share some of the things I learned along the way here...

  1. the angular universal docs suggest having a separate app.server.module.ts ... but in my case I found it pretty necessary to take it a bit further and split off the browser/cordova app module too so I can prevent certain things from getting into the server build at all (external dependencies in particular).
  2. I'm now in the process of taking THAT even further and splitting the browser and cordova modules - it looks like this is gonna shave a few hundred K off the main bundle for the browser... there's stuff I don't want in the cordova build anyway, like the service worker.
  3. along those same lines, I ended up making an injectible called PlatformService (an abstract class) and PlatformServer and PlatformCordova implementations of it (e.g. localstorage shim, onCordovaReady, getVideoRecordingPermissions, etc) - that way my application code doesn't have to care about any of that.

There may be other approaches to some of this, but after lots of trial and error this is what I settled on. Do you have any preferences?

There are other little details too related to configuratons, directories, npm scripts, etc. But those are probably minor enough to deal with in a PR.

@captaincaius
Copy link
Contributor

I just noticed I missed that there's already a working branch 👏 woohoo.

I wanted to add a couple things to the consideration of whether it's worthwhile to split the cordova and browser builds:

  1. Looks like feat(service-worker): expose RegistrationOptions token to allow for runtime configuration angular/angular#26002 is gonna get in, and it should allow the service worker to be disabled dynamically if we're in cordova even with AoT: ... it's going into 8, so I guess timeline becomes a factor.
  2. I forgot to mention some context for the build-size savings I mentioned above:
  • it was about 200k+ difference in the minified, but not gzipped size

  • it was with a moderate amount of plugins - pasting cordova and ionic native stuff in package.json below:

    "@ionic-native/android-permissions": "5.0.0-beta.21",
    "@ionic-native/camera": "^5.4.0",
    "@ionic-native/core": "5.0.0-beta.21",
    "@ionic-native/device": "5.0.0-beta.21",
    "@ionic-native/splash-screen": "5.0.0-beta.21",
    "@ionic-native/status-bar": "5.0.0-beta.21",
    "cordova-android": "^7.0.0",
    "cordova-custom-config": "^5.0.3",
    "cordova-plugin-android-permissions": "^1.0.0",
    "cordova-plugin-camera": "^4.0.3",
    "cordova-plugin-device": "^2.0.2",
    "cordova-plugin-file": "^6.0.1",
    "cordova-plugin-file-transfer": "^1.7.1",
    "cordova-plugin-ionic-keyboard": "^2.1.2",
    "cordova-plugin-ionic-webview": "^2.3.2",
    "cordova-plugin-media-capture": "^3.0.2",
    "cordova-plugin-splashscreen": "^5.0.2",
    "cordova-plugin-statusbar": "^2.4.1",
    "cordova-plugin-whitelist": "^1.3.3",

So I'm not sure the 2xxK savings is worthwhile for EVERYONE to justify the added complexity and build time after ng8 lands. But I do think having a separate module for non-server is pretty necessary for keeping misbehaving code off of the server.

@sinedied
Copy link
Member Author

sinedied commented May 9, 2019

along those same lines, I ended up making an injectible called PlatformService (an abstract class)
and PlatformServer and PlatformCordova implementations of it (e.g. localstorage shim,
onCordovaReady, getVideoRecordingPermissions, etc) - that way my application code doesn't have > to care about any of that.

I like this idea a lot! 👍

One of the initial goal of this project was to keep the same codebase running for all target, so having distinct app modules for server/browser itch me a bit, but maybe that could not be avoided.
On the other hand, having all target-specific services/class with dedicated modules in the idea of your PlatformService is really nice, as it clearly allow to separate platform specifics from the business code, while allowing to exclude the cordova dependencies from browser module and so on.

@captaincaius
Copy link
Contributor

:D believe me it itched me a lot too and I'm still applying ointment. After many unsuccessful workarounds (and 30 minute builds) to resist it, I finally gave up.

Now my QA deployments take forever, but at least I can have SSR, service worker, and cordova builds living in harmony.

I'll try to pitch in a bit on this when I have a chance.

@qliqdev
Copy link

qliqdev commented Mar 30, 2022

Any updates?

@sinedied
Copy link
Member Author

No one is working on it at the moment unfortunately.
Your best change would be to start from ng add @nguniversal/express-engine.
Now that schematics are there it should simplify a bit the base work, but the main challenge is to make sure all template paths are compatible with server side rendering, which is tricky especially with the mobile parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants