fix: getContext() should be sync #222
Conversation
✅ Deploy Preview for plugin-angular-universal-demo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
||
| ```diff | ||
| +import { REQUEST, REQUEST_CONTEXT } from '@angular/ssr/tokens' | ||
| +import { REQUEST, REQUEST_CONTEXT } from '@angular/core' |
There was a problem hiding this comment.
Ref angular/angular-cli#28871 - previous module was removed in @angular/ssr@19.0.0-rc.3
| @Inject('netlify.request') @Optional() request?: Request, | ||
| @Inject('netlify.context') @Optional() context?: Context, | ||
| @Inject(REQUEST) @Optional() request?: Request, | ||
| @Inject(REQUEST_CONTEXT) @Optional() context?: Context, |
There was a problem hiding this comment.
I forgot to convert that in previous PR (netlify.request -> REQUEST) after setting up AppEngine, so this both moves that and use new place to get REQUEST and REQUEST_CONTEXT tokens from
| @@ -7,6 +7,7 @@ | |||
| "src/**/*.js", | |||
| "src/**/*.mjs", | |||
| "src/**/*.json", | |||
| "src/**/*.d.ts", | |||
There was a problem hiding this comment.
Otherwise https://github.com/netlify/angular-runtime/blob/main/src/common-engine.d.ts and https://github.com/netlify/angular-runtime/blob/main/src/context.d.ts wouldn't be packed for npm - it worked locally with demo and test fixtures because of package symlinking, but would result in something like below outside of the repo:
Application bundle generation failed. [4.128 seconds]
✘ [ERROR] TS7016: Could not find a declaration file for module '@netlify/angular-runtime/common-engine'. '/Users/misiek/test/angular-19-rnd-1/node_modules/@netlify/angular-runtime/src/common-engine.mjs' implicitly has an 'any' type.
Try `npm i --save-dev @types/netlify__angular-runtime` if it exists or add a new declaration (.d.ts) file containing `declare module '@netlify/angular-runtime/common-engine';` [plugin angular-compiler]
src/server.ts:2:23:
2 │ import { render } from '@netlify/angular-runtime/common-engine'
╵ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| @@ -1,2 +1,2 @@ | |||
| // eslint-disable-next-line no-undef | |||
| export const getContext = async () => Netlify?.context | |||
| export const getContext = () => (typeof Netlify !== 'undefined' ? Netlify?.context : undefined) | |||
There was a problem hiding this comment.
The async was wrong and just result of copying boiler plate from other file. I did miss staging that change in previous PR, but other than that I also had to add check to see if global Netlify is defined, otherwise ng serve (Angular dev mode) would fail with:
ReferenceError: Netlify is not defined
at Module.getContext (/Users/misiek/test/angular-19-rnd-2/.angular/cache/19.0.0-rc.3/angular-19-rnd-2/vite/deps_ssr/@netlify_angular-runtime_context.js?v=61c1d17e:1:1)
at netlifyAppEngineHandler (/Users/misiek/test/angular-19-rnd-2/src/server.ts:9:19)
at /Users/misiek/test/angular-19-rnd-2/node_modules/@angular/build/src/tools/vite/middlewares/ssr-middleware.js:83:38
Here are few follow ups from #216
Most important:
getContextneed to be sync not async (forgot to stage this in previous PR) and additionally we should check for Netlify global being defined (otherwsie Angular Dev doesn't work)2nd most important: Make sure
.d.tsfiles get packed, otherwise Angular Compiler will fail (by default) because of missing declarationsI'm also updating to newest RCs released few hours ago and applying some changes based on what changed between rc.2 and rc.3 (in our readme and in demo app)