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

Add ESM wrapper to avoid dual package hazard #842

Merged
merged 10 commits into from
Oct 6, 2023
2 changes: 1 addition & 1 deletion packages/connect-web-bench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ it like a web server would usually do.

| code generator | bundle size | minified | compressed |
|----------------|-------------------:|-----------------------:|---------------------:|
| connect | 113,448 b | 49,872 b | 13,465 b |
| connect | 113,741 b | 50,030 b | 13,533 b |
timostamm marked this conversation as resolved.
Show resolved Hide resolved
| grpc-web | 414,071 b | 300,352 b | 53,255 b |
65 changes: 21 additions & 44 deletions packages/connect/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,67 +8,44 @@
"url": "https://github.com/connectrpc/connect-es.git",
"directory": "packages/connect"
},
"sideEffects": false,
"scripts": {
"clean": "rm -rf ./dist/cjs/* ./dist/esm/*",
"clean": "rm -rf ./dist/*",
"generate": "buf generate src/protocol-grpc/proto",
"build": "npm run build:cjs && npm run build:esm && node scripts/update-user-agent.mjs",
"build": "npm run build:cjs && npm run build:esm && npm run build:proxy && node scripts/update-user-agent.mjs",
"build:cjs": "tsc --project tsconfig.json --module commonjs --outDir ./dist/cjs --declaration --declarationDir ./dist/cjs && echo >./dist/cjs/package.json '{\"type\":\"commonjs\"}'",
"build:esm": "tsc --project tsconfig.json --module ES2015 --verbatimModuleSyntax --outDir ./dist/esm --declaration --declarationDir ./dist/esm && echo >./dist/esm/package.json '{\"type\":\"module\", \"sideEffects\":false}'",
"build:esm": "tsc --project tsconfig.json --module ES2015 --verbatimModuleSyntax --outDir ./dist/esm --declaration --declarationDir ./dist/esm",
"build:proxy": "node ../../scripts/gen-esm-proxy.mjs . protocol protocol-connect protocol-grpc protocol-grpc-web",
"jasmine": "jasmine --config=jasmine.json",
"attw": "attw --pack"
},
"type": "module",
"sideEffects": false,
"main": "./dist/cjs/index.js",
"exports": {
".": {
"import": {
"types": "./dist/esm/index.d.ts",
"default": "./dist/esm/index.js"
},
"require": {
"types": "./dist/cjs/index.d.ts",
"default": "./dist/cjs/index.js"
}
"module": "./dist/esm/index.js",
"require": "./dist/cjs/index.js",
"import": "./dist/proxy/index.js"
},
"./protocol": {
"import": {
"types": "./dist/esm/protocol/index.d.ts",
"default": "./dist/esm/protocol/index.js"
},
"require": {
"types": "./dist/cjs/protocol/index.d.ts",
"default": "./dist/cjs/protocol/index.js"
}
"module": "./dist/esm/protocol/index.js",
"require": "./dist/cjs/protocol/index.js",
"import": "./dist/proxy/protocol/index.js"
},
"./protocol-connect": {
"import": {
"types": "./dist/esm/protocol-connect/index.d.ts",
"default": "./dist/esm/protocol-connect/index.js"
},
"require": {
"types": "./dist/cjs/protocol-connect/index.d.ts",
"default": "./dist/cjs/protocol-connect/index.js"
}
"module": "./dist/esm/protocol-connect/index.js",
"require": "./dist/cjs/protocol-connect/index.js",
"import": "./dist/proxy/protocol-connect/index.js"
},
"./protocol-grpc": {
"import": {
"types": "./dist/esm/protocol-grpc/index.d.ts",
"default": "./dist/esm/protocol-grpc/index.js"
},
"require": {
"types": "./dist/cjs/protocol-grpc/index.d.ts",
"default": "./dist/cjs/protocol-grpc/index.js"
}
"module": "./dist/esm/protocol-grpc/index.js",
"require": "./dist/cjs/protocol-grpc/index.js",
"import": "./dist/proxy/protocol-grpc/index.js"
},
"./protocol-grpc-web": {
"import": {
"types": "./dist/esm/protocol-grpc-web/index.d.ts",
"default": "./dist/esm/protocol-grpc-web/index.js"
},
"require": {
"types": "./dist/cjs/protocol-grpc-web/index.d.ts",
"default": "./dist/cjs/protocol-grpc-web/index.js"
}
"module": "./dist/esm/protocol-grpc-web/index.js",
"require": "./dist/cjs/protocol-grpc-web/index.js",
"import": "./dist/proxy/protocol-grpc-web/index.js"
}
},
"typesVersions": {
Expand Down
3 changes: 2 additions & 1 deletion packages/connect/src/connect-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {
} from "@bufbuild/protobuf";
import { Message } from "@bufbuild/protobuf";
import { codeToString } from "./protocol-connect/code-string.js";
import { isProtobufMessage } from "./protocol/is-protobuf-message.js";

/**
* ConnectError captures four pieces of information: a Code, an error
Expand Down Expand Up @@ -152,7 +153,7 @@ export class ConnectError extends Error {
: typeOrRegistry;
const details: AnyMessage[] = [];
for (const data of this.details) {
if (data instanceof Message) {
if (isProtobufMessage(data)) {
if (registry.findMessage(data.getType().typeName)) {
details.push(data);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/connect/src/http-headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { BinaryReadOptions, MessageType } from "@bufbuild/protobuf";
import { Message, protoBase64 } from "@bufbuild/protobuf";
import { ConnectError } from "./connect-error.js";
import { Code } from "./code.js";
import { isProtobufMessage } from "./protocol/is-protobuf-message.js";

/**
* Encode a single binary header value according to the Connect
Expand All @@ -30,7 +31,7 @@ export function encodeBinaryHeader(
value: Uint8Array | ArrayBufferLike | Message | string,
): string {
let bytes: Uint8Array;
if (value instanceof Message) {
if (isProtobufMessage(value)) {
bytes = value.toBinary();
} else if (typeof value == "string") {
bytes = new TextEncoder().encode(value);
Expand Down
5 changes: 3 additions & 2 deletions packages/connect/src/protocol-connect/error-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { Message, protoBase64 } from "@bufbuild/protobuf";
import type {
JsonObject,
JsonValue,
JsonWriteOptions,
} from "@bufbuild/protobuf";
import { protoBase64 } from "@bufbuild/protobuf";
import { Code } from "../code.js";
import { ConnectError } from "../connect-error.js";
import { codeFromString, codeToString } from "./code-string.js";
import { isProtobufMessage } from "../protocol/is-protobuf-message.js";

/**
* Parse a Connect error from a JSON value.
Expand Down Expand Up @@ -132,7 +133,7 @@ export function errorToJson(
};
o.details = error.details
.map((value) => {
if (value instanceof Message) {
if (isProtobufMessage(value)) {
const i: IncomingDetail = {
type: value.getType().typeName,
value: value.toBinary(),
Expand Down
5 changes: 3 additions & 2 deletions packages/connect/src/protocol-grpc/trailer-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import { Any, Message } from "@bufbuild/protobuf";
import { Any } from "@bufbuild/protobuf";
import { Status } from "./gen/status_pb.js";
import { ConnectError } from "../connect-error.js";
import { decodeBinaryHeader, encodeBinaryHeader } from "../http-headers.js";
Expand All @@ -22,6 +22,7 @@ import {
headerGrpcStatus,
headerStatusDetailsBin,
} from "./headers.js";
import { isProtobufMessage } from "../protocol/is-protobuf-message.js";

/**
* The value of the Grpc-Status header or trailer in case of success.
Expand Down Expand Up @@ -52,7 +53,7 @@ export function setTrailerStatus(
code: error.code,
message: error.rawMessage,
details: error.details.map((value) =>
value instanceof Message
isProtobufMessage(value)
? Any.pack(value)
: new Any({
typeUrl: `type.googleapis.com/${value.type}`,
Expand Down
1 change: 1 addition & 0 deletions packages/connect/src/protocol/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export { runUnaryCall, runStreamingCall } from "./run-call.js";
// following exports, we recommend that you do so with an exact version
// constraint (no ~ or ^).

export { isProtobufMessage } from "./is-protobuf-message.js";
export {
createMethodSerializationLookup,
createClientMethodSerializers,
Expand Down
34 changes: 34 additions & 0 deletions packages/connect/src/protocol/is-protobuf-message.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2021-2023 The Connect Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import { Message } from "@bufbuild/protobuf";

/**
* Temporary workaround for dual package hazard caused by @bufbuild/protobuf
*
* @private Internal code, does not follow semantic versioning.
*/
export function isProtobufMessage<T extends Message<T>>(
timostamm marked this conversation as resolved.
Show resolved Hide resolved
arg: string | Uint8Array | ArrayBufferLike | Message<T> | object,
): arg is Message<T> {
return (
typeof arg == "object" &&
"toJson" in arg &&
typeof arg.toJson == "function" &&
"toBinary" in arg &&
typeof arg.toBinary == "function" &&
"getType" in arg &&
typeof arg.getType == "function"
);
}
3 changes: 2 additions & 1 deletion packages/connect/src/protocol/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import type { PartialMessage, MessageType } from "@bufbuild/protobuf";
import { Message } from "@bufbuild/protobuf";
import { isProtobufMessage } from "./is-protobuf-message.js";

/**
* Takes a partial protobuf messages of the
Expand All @@ -23,7 +24,7 @@ export function normalize<T extends Message<T>>(
type: MessageType<T>,
message: T | PartialMessage<T>,
) {
return message instanceof Message ? message : new type(message);
return isProtobufMessage(message) ? message : new type(message);
}

/**
Expand Down
101 changes: 101 additions & 0 deletions scripts/gen-esm-proxy.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import {existsSync, mkdirSync, readFileSync, writeFileSync} from "node:fs";
import * as path from "node:path";
import {exit, stderr, stdout, argv} from "node:process";

const args = argv.slice(2);

if (args.length === 0) {
stdout.write(`USAGE: ${path.basename(argv[1])} [...path]
path: One or more subpath exports. A dot (.) is valid.

Generates ESM wrappers for dual packages.

Example:

${path.basename(argv[1])} . foo

Assumes that the following CJS artifacts exist:

dist/cjs
├── index.d.ts
├── index.js
└── foo
├── index.d.ts
└── index.js

Generates the following ESM wrappers:

dist/proxy
├── index.d.ts
├── index.js
└── foo
├── index.d.ts
└── index.js

package.json must contain:

"type": "module",
"exports": {
".": {
"require": "./dist/cjs/index.js",
"import": "./dist/esmfake/index.js"
},
"./protocol": {
"require": "./dist/cjs/foo/index.js",
"import": "./dist/esmfake/foo/index.js"
},

Known limitations:
- expects CJS files with a .js extension, not .cjs
- does not support default exports
- supports only simple subpath directory exports, assuming a index.js file
- does not support export patterns
`);
exit(1);
}

const cjsDist = "dist/cjs";
const proxyDist = "dist/proxy";

const packageType = readPackageJsonType();
if (packageType !== "module") {
stderr.write(`package.json is missing "type": "module" field.\n`);
exit(1);
}

for (const subpath of args) {
const cjsPath = path.join(cjsDist, subpath, "index.js");
if (!existsSync(cjsPath)) {
stderr.write(`CommonJS artifact for subpath "${subpath}" not found at expected path ${cjsPath}\n`);
exit(1);
}
const proxyDir = path.join(proxyDist, subpath);
if (!existsSync(proxyDir)) {
mkdirSync(proxyDir, { recursive: true });
}
const cjsImportPath = path.relative(proxyDir, cjsPath)
writeFileSync(
path.join(proxyDir, "index.js"),
`export * from "${cjsImportPath}";\n`,
);
writeFileSync(
path.join(proxyDir, "index.d.ts"),
`export * from "${cjsImportPath}";\n`,
);
}

/**
* @return {"commonjs"|"module"}
*/
function readPackageJsonType() {
const pkgString = readFileSync("package.json", "utf-8");
const pkg = JSON.parse(pkgString);
const t = pkg.type;
if (typeof t !== "string") {
return "commonjs";
}
if (t.toLowerCase() === "module") {
return "module";
}
return "commonjs";
}