feat(schematics): firebase function deployment for angular universal #2305
Conversation
|
I'm fine with it being one big commit. Don't see any obvious red flags, I'll give it a proper review and a test drive come Monday. Will let you know how that goes. Thanks! |
|
Simple case seems to be working well. Going through the code. What do you think of making the deploy process for Universal like this by default though:
That way all the static assets / prerenders are on the CDN w/o need for cold-start. We could also give the developer some options here:
|
|
Currently, the generated {
"target": "project",
"public": "dist/browser",
"ignore": ["firebase.json", "**/.*", "**/node_modules/**"],
"rewrites": [
{
"source": "**",
"function": "ssr"
}
]
}
}My understanding is that we deploy static assets to a CDN and we have the |
|
Ok, fair enough on the prerender; we can always see if that's requested later down the road.
That said, I missed that you were already deploying the browser bundle to hosting. So ignore that feedback |
| "logs": "firebase functions:log" | ||
| }, | ||
| "engines": { | ||
| "node": "8" |
jamesdaniels
Feb 5, 2020
Member
It's beta, but what do you think of defaulting to Node 10? I think it would be a better experience out of box.
jamesdaniels
Feb 5, 2020
Member
Let's warn/error if the engine version doesn't match their local development.
| "node": "8" | ||
| }, | ||
| "dependencies": { | ||
| "firebase-admin": "~7.0.0", |
jamesdaniels
Feb 5, 2020
Member
Can we make these versions configurable, maybe look at their dev deps?
| "engines": { | ||
| "node": "8" | ||
| }, | ||
| "dependencies": { |
jamesdaniels
Feb 5, 2020
Member
Is there a way we could allow configuration of webpack externals here? E.g, libs that they don't want bundled but use in SSR
|
Also general feedback, the deploy was less interactive than I would've liked. Can we pipe the output from the hosting / functions or put our own spinner / progress indicator? |
|
LGTM, comments are nits. Feel free to address before I cut rc.1 or we can take as action items. |
|
Thanks for the review! I'll address some of the comments later this week. I'll probably not find the time for everything we discussed last week, but it shouldn't be a blocker for 6.0.0. |
|
@jamesdaniels I did a few improvements:
|
This PR includes: 1. Refactoring of the `ng-add` schematics. It decomposes the function to two separate ones responsible for static file deployments and SSR. Unfortunately, I wasn't able to get rid of the extra schematic from `collection.json` since currently our APIs do not allow manually persisting the `Tree` on the disk. 2. Minor refactoring of the `deploy` builder to incorporate the functionality for server-side rendering enabled deployments. 3. Refactoring of the tests to reflect the updated structure of `ng-add` and the deploy action. 4. Implementation of deployment to Firebase functions. This implementation supports Angular Universal version 9 and above. Originally I was thinking of checking the dependency versions manually with `semver` during `ng deploy`/`ng add`, but then decided that the peer dependency check that `@angular/fire` does might be sufficient. Since the PR is relatively big, if you prefer I can decompose it into several smaller ones.
882b254
into
angular:master
|
@mgechev @jamesdaniels hey thanks for the great work! I run ng add @angular/fire for an angular universal project (here's the result commit michelepatrassi/ng-starter@370526c) and I cannot grasp two changes:
What are these changes solving? Does the second one maybe solves something like the externals filter done here https://gist.github.com/michelepatrassi/242f1f0a867af99918977ea64787fcee#file-webpack-server-config-js ? |

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

Checklist
yarn install,yarn testrun successfully? yesDescription
This PR includes:
ng-addschematics. It decomposes the function to two separate ones responsible for static file deployments and SSR. Unfortunately, I wasn't able to get rid of the extra schematic fromcollection.jsonsince currently our APIs do not allow manually persisting theTreeon the disk.deploybuilder to incorporate the functionality for server-side rendering enabled deployments.ng-addand the deploy action.This implementation supports Angular Universal version 9 and above. Originally I was thinking of checking the dependency versions manually with
semverduringng deploy/ng add, but then decided that the peer dependency check that@angular/firedoes might be sufficient.Since the PR is relatively big, if you prefer I can decompose it into several smaller ones.