-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add TypeScript samples support #3029
Conversation
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.
I think that owlbot-nodejs-mono-repo will have some unintended consequences, because it does things like updating the README.md and copying over templates.
What we can do is create a trimmed down version here that strips out the logic for templates, etc., and keeps just the typeless-sample bits.
Here is the summary of changes. You are about to add 2 region tags.
You are about to delete 1 region tag.
This comment is generated by snippet-bot.
|
"start": "node ./index.js", | ||
"gcp-build": "tsc -p .", |
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.
Was this removed intentionally, the documentation still refers to this as an option for running an custom build script: https://cloud.google.com/appengine/docs/standard/nodejs/running-custom-build-step#example
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.
I am pretty sure this was a mistake. The docs are correct.
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 actually broke the ability to deploy to GAE. It now fails in the build phase with an error like:
ERROR: (gcloud.app.deploy) Error Response: [9] Cloud build cdec98ca-e4aa-4a97-bbfc-d5843c7ff8a3 status: FAILURE
> appengine-typescript@0.0.1 prepare
> npm run compile
> appengine-typescript@0.0.1 compile
> tsc -p .
sh: 1: tsc: not found
npm ERR! code 127
npm ERR! path /workspace
npm ERR! command failed
npm ERR! command sh -c -- npm run compile
npm ERR! A complete log of this run can be found in:
This is because the devDependencies
are not installed when the prepare script is run. The gcp-build
script fixes this.
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 has been reverted, and a test with npm run deploy
was successful.
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.
FYI, we are currently working on a change to run npm run build
by default when deploying to AppEngine, because this gcp-build
thing is not very intuitive.
cc @anniefu
This PR enables adding TypeScript samples without managing duplicate JS and TS versions.
owlbot.py
runs the bot and updates the formatting. It bypasses theowlbot_main()
flow that performs client library tasks.scheduler
was included to test the process. Also, the existingappengine/typescript
sample needed to be updated to reconcile with the root level.eslintrc.json
.