Skip to content
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

fix: Use commonjs_strict import_type to avoid Functions in generated code #182

Merged
merged 3 commits into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
```
74 changes: 61 additions & 13 deletions javascript-web/generate_protos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,27 @@
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
sed_command="sed -i ''"
elif [[ "$OSTYPE" == "linux"* ]]; then
platform='linux-x86_64'
elif [ $1 == 'windows' ]; then
sed_command="sed -i"
elif [[ "$OSTYPE" == "cygwin"* || "$OSTYPE" == "msys"* ]]; then
platform='windows-x86_64'
sed_command="sed -i"
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'
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 +x $plugin
chmod a+x $plugin

out=./src
rm -rf $out
Expand All @@ -32,10 +33,57 @@ 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 "Backing up protos dir"
cp -r ../proto ../proto.bak

echo "Commenting out package declarations"
for f in $proto_file_list
do
$sed_command 's/^\s*package \(.*\)/\/\/package \1/g' ../proto/${f}
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 "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 \
extensions.proto cacheclient.proto controlclient.proto auth.proto cacheping.proto cachepubsub.proto
${proto_file_list}
19 changes: 16 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.

4 changes: 3 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,7 @@
"typescript": "4.4.3"
},
"dependencies": {
"grpc-web": "1.4.2"
},
"files": [
"dist"
Expand Down