angular build adatper draft #139
Conversation
first draft of angular build adapter code
There was a problem hiding this comment.
Could you add unit tests as well? similar to https://github.com/FirebaseExtended/firebase-framework-tools/blob/main/packages/%40apphosting/adapter-nextjs/src/bin/build.spec.ts
| // TODO do all the things | ||
| console.log({ config }); | ||
| const outputBundleOptions = populateOutputBundleOptions(); | ||
| await build().catch(() => process.exit(1)); |
There was a problem hiding this comment.
Whats the intention of catching and then doing process.exit?
There was a problem hiding this comment.
The 'spawn' event will fire regardless of whether an error occurs within the spawned process. So in case reject(), we can catch and exit with 1
refactor to move helpers to utils.ts, and add interface.ts
enable angular build logs to obtain the manifest, which contains outputpaths and a build summary.
Remove manually loading angular application builder options & normalizing options Use build manifest from stdout, verify its format with zod, and then extract the output paths. Errors and warnings from the manifest will be logged.
|
Removed manually loading angular application builder options & normalizing options |
| @@ -13,6 +13,7 @@ module.exports = { | |||
| ], | |||
| rules: { | |||
| "jsdoc/newline-after-description": "off", | |||
| "@typescript-eslint/no-explicit-any": "off", // TODO(sijin): remove later | |||
There was a problem hiding this comment.
to do the zod custom url type I have to use type any, couldn't figure out how to make it work without turning off "no-explicit-any"
There was a problem hiding this comment.
You can add a eslint-disable-next-line comment above the line in question, rather than disable the rule
There was a problem hiding this comment.
btw unknown is an alternative to any that doesn't require changing the ESLint rules
| manifest.errors.forEach((error) => { | ||
| logger.error(error); | ||
| }); | ||
| reject(); |
There was a problem hiding this comment.
Should probably be an error inside of reject(). This will help for debugging in the future go/tsjs-style#only-throw-errors
| if (manifest["outputPaths"]) { | ||
| outputPathOptions = populateOutputBundleOptions(manifest["outputPaths"]); | ||
| } else { | ||
| throw new Error("Could not find output paths from the build manifest."); |
There was a problem hiding this comment.
In the case where this error is thrown and then caught in line 108, what happens to outputPathOptions? Is it {} still? Does that work still?
There was a problem hiding this comment.
Outputpaths should always be there, so I fixed this part. This manifest should be validated by zod and outputPathOptions should not be empty
| @@ -14,6 +14,7 @@ | |||
| "packages/**/*" | |||
| ], | |||
| "devDependencies": { | |||
| "@angular-devkit/build-angular": "^17.1.2", | |||
There was a problem hiding this comment.
can you move this to the package.json in packages/@apphosting/adapter-angular
| @@ -25,6 +26,7 @@ | |||
| "mocha": "^10.2.0", | |||
| "prettier": "^3.0.3", | |||
| "ts-mocha": "^10.0.0", | |||
| "typescript": "^5.2.0" | |||
| "typescript": "^5.2.0", | |||
| "zod": "^3.22.4" | |||
There was a problem hiding this comment.
also move to package.json in packages/@apphosting/adapter-angular
| } | ||
| }); | ||
| } else { | ||
| throw new Error("Unable to locate build manifest with output paths."); |
There was a problem hiding this comment.
let's not throw here, ignore since the SSG / other build scripts might be emitting to stdout
| shell: true, | ||
| stdio: ["inherit", "pipe", "pipe"], | ||
| }); | ||
| let outputPathOptions = {} as OutputPathOptions; |
There was a problem hiding this comment.
instead, let's make this let outputPathOptions: OutputPathOptions | undefined; and on exit, throw is outputPathOptions is still undefined
angular build adapter script
output directory structure & bundle.yaml file after running adapter:
