From 8ed90a73d46679eb3dba124edfa1ce2e90de9248 Mon Sep 17 00:00:00 2001 From: Ramya Krishnamoorthy Date: Wed, 2 Aug 2023 13:57:55 -0700 Subject: [PATCH 1/3] fix: Use commonjs_strict import_type to avoid Functions in generated code --- javascript-web/README.md | 2 +- javascript-web/generate_protos.sh | 78 +++++++++++++++++++++++++------ javascript-web/package-lock.json | 30 ++++++++++-- javascript-web/package.json | 5 +- 4 files changed, 96 insertions(+), 19 deletions(-) diff --git a/javascript-web/README.md b/javascript-web/README.md index bbb2c14..125fb06 100644 --- a/javascript-web/README.md +++ b/javascript-web/README.md @@ -36,5 +36,5 @@ if (getResult.getResult() === ECacheResult.HIT) { # Generating definitions ```bash -./generate_protos.sh [osx, linux, windows] +./generate_protos.sh ``` diff --git a/javascript-web/generate_protos.sh b/javascript-web/generate_protos.sh index 86d40f8..5af364b 100755 --- a/javascript-web/generate_protos.sh +++ b/javascript-web/generate_protos.sh @@ -2,26 +2,22 @@ set -e set -x -if [ $# -lt 1 ]; then - echo usage: $0 osx [osx, linux, windows] - exit 1 -fi -if [ $1 == 'osx' ]; then +if [[ "$OSTYPE" == "darwin"* ]]; then platform='darwin-x86_64' -elif [ $1 == 'linux' ]; then +elif [[ "$OSTYPE" == "linux"* ]]; then platform='linux-x86_64' -elif [ $1 == 'windows' ]; then +elif [[ "$OSTYPE" == "cygwin"* || "$OSTYPE" == "msys"* ]]; then platform='windows-x86_64' else - echo usage: $0 osx [osx, linux, windows] + echo "Unknown OS: $OSTYPE" exit 1 fi version='1.3.1' plugin='/usr/local/bin/protoc-gen-grpc-web' -rm -f $plugin -curl -L https://github.com/grpc/grpc-web/releases/download/$version/protoc-gen-grpc-web-$version-$platform -o $plugin -chmod +x $plugin +sudo rm -f $plugin +sudo curl -L https://github.com/grpc/grpc-web/releases/download/$version/protoc-gen-grpc-web-$version-$platform -o $plugin +sudo chmod a+x $plugin out=./src rm -rf $out @@ -32,10 +28,64 @@ mkdir $out # brew link --overwrite protobuf@3 # https://github.com/protocolbuffers/protobuf-javascript/issues/127#issuecomment-1204202844 +# The `--js_out` plugin will generate JavaScript code (`echo_pb.js`), and the +# `-grpc-web_out` plugin will generate a TypeScript definition file for it +# (`echo_pb.d.ts`). This is a temporary hack until the `--js_out` supports +# TypeScript itself. See https://github.com/grpc/grpc-web/blob/7c528784576abbbfd05eb6085abb8c319d76ab05/README.md?plain=1#L246 + +# Ramya: We need strict commonjs to allow Cloudflare workers to run the web sdk. +# After changing to commonjs_strict, the web SDK will build but unit tests and integ tests fail with +# ``` +# Test suite failed to run +# TypeError: Cannot read properties of undefined (reading 'Never') +# +# 7 | } from '@gomomento/generated-types-webtext/dist/auth_pb'; +# 8 | import {cacheServiceErrorMapper} from '../errors/cache-service-error-mapper'; +# > 9 | import Never = _GenerateApiTokenRequest.Never; +# | ^ +# 10 | import Expires = _GenerateApiTokenRequest.Expires; +# 11 | import { +# 12 | CredentialProvider, + +# at Object. (src/internal/auth-client.ts:9:41) +# at Object. (src/auth-client.ts:5:1) +# at Object. (src/index.ts:2:1) +# at Object. (test/integration/integration-setup.ts:13:1) +# at Object. (test/integration/shared/auth-client.test.ts:2:1) +# ``` +# I believe this is related to https://github.com/protocolbuffers/protobuf-javascript/issues/40 +# From a hint in https://medium.com/expedia-group-tech/the-weird-world-of-grpc-tooling-for-node-js-part-2-daafed94cc32, +# I found that removing the `package ` from the protos gives us the JS exports we want. +# Removing the package permanently is not possible now because it is part of the GRPC method descriptor. +# So we do a terrible hack to comment out the package declaration before generating the JS types, +# but add them back before generating the GRPC web bindings + +proto_file_list=" extensions.proto cacheclient.proto controlclient.proto auth.proto cacheping.proto cachepubsub.proto " + +echo "Commenting out package declarations" +for f in $proto_file_list +do + if [[ "$OSTYPE" == "darwin"* ]]; then + sed -i '' 's/^\s*package \(.*\)/\/\/package \1/g' ../proto/${f} + else + sed -i 's/^\s*package \(.*\)/\/\/package \1/g' ../proto/${f} + fi +done + protoc -I=../proto -I=/usr/local/include \ - --js_out=import_style=commonjs:$out \ - extensions.proto cacheclient.proto controlclient.proto auth.proto cacheping.proto cachepubsub.proto + --js_out=import_style=commonjs_strict:$out \ + ${proto_file_list} + +echo "Uncommenting package declarations" +for f in $proto_file_list +do + if [[ "$OSTYPE" == "darwin"* ]]; then + sed -i '' 's/^\/\/package \(.*\)/package \1/g' ../proto/${f} + else + sed -i 's/^\/\/package \(.*\)/package \1/g' ../proto/${f} + fi +done protoc -I=../proto -I=/usr/local/include \ --grpc-web_out=import_style=typescript,mode=grpcwebtext:$out \ - extensions.proto cacheclient.proto controlclient.proto auth.proto cacheping.proto cachepubsub.proto + ${proto_file_list} diff --git a/javascript-web/package-lock.json b/javascript-web/package-lock.json index 738bdda..02d91c5 100644 --- a/javascript-web/package-lock.json +++ b/javascript-web/package-lock.json @@ -1,13 +1,17 @@ { - "name": "@gomomento/generated-types", + "name": "@gomomento/generated-types-webtext", "version": "0.0.1", "lockfileVersion": 2, "requires": true, "packages": { "": { - "name": "@gomomento/generated-types", + "name": "@gomomento/generated-types-webtext", "version": "0.0.1", - "license": "MIT", + "license": "Apache-2.0", + "dependencies": { + "google-protobuf": "3.21.2", + "grpc-web": "1.4.2" + }, "devDependencies": { "@tsconfig/node16": "1.0.2", "@types/node": "16.10.3", @@ -26,6 +30,16 @@ "integrity": "sha512-ho3Ruq+fFnBrZhUYI46n/bV2GjwzSkwuT4dTf0GkuNFmnb8nq4ny2z9JEVemFi6bdEJanHLlYfy9c6FN9B9McQ==", "dev": true }, + "node_modules/google-protobuf": { + "version": "3.21.2", + "resolved": "https://registry.npmjs.org/google-protobuf/-/google-protobuf-3.21.2.tgz", + "integrity": "sha512-3MSOYFO5U9mPGikIYCzK0SaThypfGgS6bHqrUGXG3DPHCrb+txNqeEcns1W0lkGfk0rCyNXm7xB9rMxnCiZOoA==" + }, + "node_modules/grpc-web": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/grpc-web/-/grpc-web-1.4.2.tgz", + "integrity": "sha512-gUxWq42l5ldaRplcKb4Pw5O4XBONWZgz3vxIIXnfIeJj8Jc3wYiq2O4c9xzx/NGbbPEej4rhI62C9eTENwLGNw==" + }, "node_modules/typescript": { "version": "4.4.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.4.3.tgz", @@ -53,6 +67,16 @@ "integrity": "sha512-ho3Ruq+fFnBrZhUYI46n/bV2GjwzSkwuT4dTf0GkuNFmnb8nq4ny2z9JEVemFi6bdEJanHLlYfy9c6FN9B9McQ==", "dev": true }, + "google-protobuf": { + "version": "3.21.2", + "resolved": "https://registry.npmjs.org/google-protobuf/-/google-protobuf-3.21.2.tgz", + "integrity": "sha512-3MSOYFO5U9mPGikIYCzK0SaThypfGgS6bHqrUGXG3DPHCrb+txNqeEcns1W0lkGfk0rCyNXm7xB9rMxnCiZOoA==" + }, + "grpc-web": { + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/grpc-web/-/grpc-web-1.4.2.tgz", + "integrity": "sha512-gUxWq42l5ldaRplcKb4Pw5O4XBONWZgz3vxIIXnfIeJj8Jc3wYiq2O4c9xzx/NGbbPEej4rhI62C9eTENwLGNw==" + }, "typescript": { "version": "4.4.3", "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.4.3.tgz", diff --git a/javascript-web/package.json b/javascript-web/package.json index a3ae355..f5aaf7c 100644 --- a/javascript-web/package.json +++ b/javascript-web/package.json @@ -10,7 +10,8 @@ }, "scripts": { "test": "echo \"Error: no test specified\" && exit 1", - "build": "rm -rf dist && mkdir dist && ./generate_protos.sh linux && cp index.ts src/ && tsc && cp src/*.d.ts ./dist && cp src/*.js ./dist" + "build": "rm -rf dist && mkdir dist && ./generate_protos.sh && cp index.ts src/ && tsc && cp src/*.d.ts ./dist && cp src/*.js ./dist", + "build-only": "rm -rf dist && mkdir dist && cp index.ts src/ && tsc && cp src/*.d.ts ./dist && cp src/*.js ./dist" }, "author": "", "license": "Apache-2.0", @@ -20,6 +21,8 @@ "typescript": "4.4.3" }, "dependencies": { + "google-protobuf": "3.21.2", + "grpc-web": "1.4.2" }, "files": [ "dist" From 8584f7061139ad0a06f84e864e2d4e0e8763f20e Mon Sep 17 00:00:00 2001 From: Chris Price Date: Thu, 3 Aug 2023 05:52:49 -0700 Subject: [PATCH 2/3] chore: minor simplifications to js-web proto generation process --- javascript-web/generate_protos.sh | 34 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/javascript-web/generate_protos.sh b/javascript-web/generate_protos.sh index 5af364b..6bad5bc 100755 --- a/javascript-web/generate_protos.sh +++ b/javascript-web/generate_protos.sh @@ -4,20 +4,25 @@ set -x if [[ "$OSTYPE" == "darwin"* ]]; then platform='darwin-x86_64' + sed_command="sed -i ''" elif [[ "$OSTYPE" == "linux"* ]]; then platform='linux-x86_64' + sed_command="sed -i" elif [[ "$OSTYPE" == "cygwin"* || "$OSTYPE" == "msys"* ]]; then platform='windows-x86_64' + sed_command="sed -i" else echo "Unknown OS: $OSTYPE" exit 1 fi version='1.3.1' -plugin='/usr/local/bin/protoc-gen-grpc-web' -sudo rm -f $plugin -sudo curl -L https://github.com/grpc/grpc-web/releases/download/$version/protoc-gen-grpc-web-$version-$platform -o $plugin -sudo chmod a+x $plugin +mkdir -p $HOME/bin +export PATH=$PATH:$HOME/bin +plugin="$HOME/bin/protoc-gen-grpc-web" +rm -f $plugin +curl -L https://github.com/grpc/grpc-web/releases/download/$version/protoc-gen-grpc-web-$version-$platform -o $plugin +chmod a+x $plugin out=./src rm -rf $out @@ -62,29 +67,22 @@ mkdir $out proto_file_list=" extensions.proto cacheclient.proto controlclient.proto auth.proto cacheping.proto cachepubsub.proto " +echo "Backing up protos dir" +cp -r ../proto ../proto.bak + echo "Commenting out package declarations" for f in $proto_file_list do - if [[ "$OSTYPE" == "darwin"* ]]; then - sed -i '' 's/^\s*package \(.*\)/\/\/package \1/g' ../proto/${f} - else - sed -i 's/^\s*package \(.*\)/\/\/package \1/g' ../proto/${f} - fi + $sed_command 's/^\s*package \(.*\)/\/\/package \1/g' ../proto/${f} done protoc -I=../proto -I=/usr/local/include \ --js_out=import_style=commonjs_strict:$out \ ${proto_file_list} -echo "Uncommenting package declarations" -for f in $proto_file_list -do - if [[ "$OSTYPE" == "darwin"* ]]; then - sed -i '' 's/^\/\/package \(.*\)/package \1/g' ../proto/${f} - else - sed -i 's/^\/\/package \(.*\)/package \1/g' ../proto/${f} - fi -done +echo "Restoring backed up protos" +rm -rf ../proto +mv ../proto.bak ../proto protoc -I=../proto -I=/usr/local/include \ --grpc-web_out=import_style=typescript,mode=grpcwebtext:$out \ From 948617042b82fa0d1b730bf45a5f288bd0bc4c80 Mon Sep 17 00:00:00 2001 From: Chris Price Date: Sat, 5 Aug 2023 07:46:24 -0700 Subject: [PATCH 3/3] chore: remove unnecessary dep --- javascript-web/package-lock.json | 11 ----------- javascript-web/package.json | 1 - 2 files changed, 12 deletions(-) diff --git a/javascript-web/package-lock.json b/javascript-web/package-lock.json index 02d91c5..578fdc8 100644 --- a/javascript-web/package-lock.json +++ b/javascript-web/package-lock.json @@ -9,7 +9,6 @@ "version": "0.0.1", "license": "Apache-2.0", "dependencies": { - "google-protobuf": "3.21.2", "grpc-web": "1.4.2" }, "devDependencies": { @@ -30,11 +29,6 @@ "integrity": "sha512-ho3Ruq+fFnBrZhUYI46n/bV2GjwzSkwuT4dTf0GkuNFmnb8nq4ny2z9JEVemFi6bdEJanHLlYfy9c6FN9B9McQ==", "dev": true }, - "node_modules/google-protobuf": { - "version": "3.21.2", - "resolved": "https://registry.npmjs.org/google-protobuf/-/google-protobuf-3.21.2.tgz", - "integrity": "sha512-3MSOYFO5U9mPGikIYCzK0SaThypfGgS6bHqrUGXG3DPHCrb+txNqeEcns1W0lkGfk0rCyNXm7xB9rMxnCiZOoA==" - }, "node_modules/grpc-web": { "version": "1.4.2", "resolved": "https://registry.npmjs.org/grpc-web/-/grpc-web-1.4.2.tgz", @@ -67,11 +61,6 @@ "integrity": "sha512-ho3Ruq+fFnBrZhUYI46n/bV2GjwzSkwuT4dTf0GkuNFmnb8nq4ny2z9JEVemFi6bdEJanHLlYfy9c6FN9B9McQ==", "dev": true }, - "google-protobuf": { - "version": "3.21.2", - "resolved": "https://registry.npmjs.org/google-protobuf/-/google-protobuf-3.21.2.tgz", - "integrity": "sha512-3MSOYFO5U9mPGikIYCzK0SaThypfGgS6bHqrUGXG3DPHCrb+txNqeEcns1W0lkGfk0rCyNXm7xB9rMxnCiZOoA==" - }, "grpc-web": { "version": "1.4.2", "resolved": "https://registry.npmjs.org/grpc-web/-/grpc-web-1.4.2.tgz", diff --git a/javascript-web/package.json b/javascript-web/package.json index f5aaf7c..f80c80d 100644 --- a/javascript-web/package.json +++ b/javascript-web/package.json @@ -21,7 +21,6 @@ "typescript": "4.4.3" }, "dependencies": { - "google-protobuf": "3.21.2", "grpc-web": "1.4.2" }, "files": [