Skip to content

Commit

Permalink
fix: Use commonjs_strict import_type to avoid Functions in generated …
Browse files Browse the repository at this point in the history
…code
  • Loading branch information
krispraws committed Aug 3, 2023
1 parent 5cc1163 commit 8ed90a7
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 19 deletions.
2 changes: 1 addition & 1 deletion javascript-web/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ if (getResult.getResult() === ECacheResult.HIT) {

# Generating definitions
```bash
./generate_protos.sh [osx, linux, windows]
./generate_protos.sh
```
78 changes: 64 additions & 14 deletions javascript-web/generate_protos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.<anonymous> (src/internal/auth-client.ts:9:41)
# at Object.<anonymous> (src/auth-client.ts:5:1)
# at Object.<anonymous> (src/index.ts:2:1)
# at Object.<anonymous> (test/integration/integration-setup.ts:13:1)
# at Object.<anonymous> (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 <pkg-name>` 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}
30 changes: 27 additions & 3 deletions javascript-web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion javascript-web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -20,6 +21,8 @@
"typescript": "4.4.3"
},
"dependencies": {
"google-protobuf": "3.21.2",
"grpc-web": "1.4.2"
},
"files": [
"dist"
Expand Down

0 comments on commit 8ed90a7

Please sign in to comment.