-
Notifications
You must be signed in to change notification settings - Fork 70
chore(examples): migrate SSR example to angular 7 #471
Conversation
@@ -0,0 +1,141 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the new .angular-cli.json
introduced by the migration to @angular/cli@^7
Deploy preview for angular-instantsearch ready! Built with commit 413c32d https://deploy-preview-471--angular-instantsearch.netlify.com |
@@ -39,7 +39,6 @@ | |||
|
|||
/** Evergreen browsers require these. **/ | |||
import 'core-js/es6/reflect'; | |||
import 'core-js/es7/reflect'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was removed to the migration.
It's is expected when upgrading to v7
@@ -1 +1 @@ | |||
export const VERSION = "2.2.1"; | |||
export const VERSION = '2.2.1'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change seems unrelated (but still good)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's just prettier doing its thing
@@ -1,6 +1,7 @@ | |||
{ | |||
"compileOnSave": false, | |||
"compilerOptions": { | |||
"importHelpers": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new requirement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is another result of the migration.
In package.json you'll also notice a new dependency [tslib].(https://www.npmjs.com/package/tslib) introduced by the migrator (I think it's optional)
This compile option is a usage requirement of this lib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep it look like it's the same purpose than Babel runtime.
@tkrugg rather than "WIP xxx" use the draft pull request! |
@@ -1,6 +1,6 @@ | |||
import 'zone.js/dist/zone-node'; | |||
import 'reflect-metadata'; | |||
import { renderModuleFactory } from '@angular/platform-server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was unused
@@ -4,25 +4,24 @@ const path = require('path'); | |||
const webpack = require('webpack'); | |||
|
|||
module.exports = { | |||
mode: 'development', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a workaround for this issue: angular/angular-cli#8616.
This is turning off some optimisation and getting rid of the TypeError: StaticInjectorError[InjectionToken DocumentToken]: StaticInjectorError[InjectionToken DocumentToken]:
error.
Note that it only affects the optimisation of the code that runs server side. The code that is shipped to the browser is optimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be nice to add this explanation in the code itself, so that next time we come across this line we know why it's here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes will do!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works on my machine
* chore(tests): add tests for ssr * separate build steps for readability * remove caret ^ from devDeps * fix restore externals in webpack.config
Summary
We used to have a hard time getting SSR to work ever since Angular 6 came out.
I've updated the project to
@angular/*
and@nguniversal
^7. It seems better.Result
I'll have to do the same thing again with Angular 6 just to check if we can completely remove any compatibility caveat from our docs, but I won't be making a PR for that
closes #379