chore: Sample CAP transpilation #273
Conversation
ZhongpinWang
force-pushed
the
fix-sample-cap-build
branch
from
28e71e1
to
0c1c45d
Compare
ZhongpinWang
changed the title
ZhongpinWang
requested review from
deekshas8,
kay-schmitteckert and
jjtang1985
| @@ -31,7 +31,7 @@ | |||
| "sample-code": "pnpm -F=@sap-ai-sdk/sample-code", | |||
| "sample-cap": "pnpm -F=@sap-ai-sdk/sample-cap", | |||
| "check:public-api": "pnpm -r check:public-api", | |||
| "check:deps": "pnpm -r -F !./tests/smoke-tests -F !./tests/schema-tests exec depcheck --ignores=\"nock,@jest/globals\" --quiet" | |||
| "check:deps": "pnpm -r -F !./tests/smoke-tests -F !./tests/schema-tests -F !./sample-cap exec depcheck --ignores=\"nock,@jest/globals\" --quiet" | |||
There was a problem hiding this comment.
Fyi, I removed deps check for this sample cap package because, e.g., xssec was added when using cds add xsuaa but our check complains that this package is not used. And also for dev deps @cap-js/cds-types.
| }, | ||
| "scripts": { | ||
| "prebuild": "rm -rf dist", | ||
| "build": "pnpm cds-build && pnpm compile && pnpm cleanup", | ||
| "cds-build": "cds build --production", |
There was a problem hiding this comment.
Fyi, cds build only generates (copies) .ts file. We need to run tsc to transpile ts files into js and remove ts files afterwards.
| "cds-build": "cds build --production", | ||
| "compile": "tsc", | ||
| "cleanup": "rm -f ./dist/srv/srv/**/*.ts ./dist/srv/package-lock.json", | ||
| "postbuild": "pushd ./dist/srv && npm i --package-lock-only && popd", |
There was a problem hiding this comment.
We have to remove the generated (copied) lock file and regenerate it in ./dist/srv using npm directly as for remote deployment, buildpack uses npm instead of pnpm. Otherwise, the lock file would only resolves into a bunch of symbolic links and a incomplete install when using buildpack.
| @@ -1,4 +1,5 @@ | |||
| @path: 'ai-api' | |||
| @requires: 'any' | |||
There was a problem hiding this comment.
Fyi, nodejs CAP by default authenticate all endpoints. Ideally, setting up an app-router to do authentication would be the right approach. For testing purpose, I just expose them all publicly.
STOP THE APPLICATION after testing.
There was a problem hiding this comment.
I would document this in the readme, as this is related to security topics.
|
@kay-schmitteckert I have to remove Issue already opened as mentioned above in the PR description. I really don't want to add too much workaround in our repository just for a known CAP issue related to types. Once it is fixed there, we can also update in our repo. |
| @@ -1,4 +1,5 @@ | |||
| @path: 'ai-api' | |||
| @requires: 'any' | |||
There was a problem hiding this comment.
I would document this in the readme, as this is related to security topics.
Context
AI/gen-ai-hub-sdk-js-backlog#160.
This PR configures the
sample-capapp, so that CAP can be transpiled into ESM JavaScript. It also provides remote deployment details.As a result, we confirm that CAP can be used together with our SDK.
Known Issue
Currently, I have to stop using
@cap-js/cds-typesfor type reference since sometimes, even after installing@sap/cdsand@cap-js/cds-types(even not as dev deps), I see the following error:This will also sometimes fail the
check:public-apiscript as the package installs a symlink in the root directory pointing to a missing target.A potential workaround could be using
pnpm rebuildto re-link the missing@cap-js/cds-typespackage. But this would require adding workaround code to our pipeline just for this sample package.An issue was already reported -> cap-js/cds-types#294 (comment).
Also reported here -> cap-js/cds-types#251 (comment)
Definition of Done
[ ] Code is tested (Unit, E2E)[ ] Error handling created / updated & covered by the tests above[ ] (Optional) Aligned changes with the Java SDK[ ] (Optional) Release notes updated