Skip to content

Commit be1eb2c

Browse files
authored
Remove use of deprecated grpc.load() method (#1669)
* Workaround for deprecated grpc.load() method * Update to protobufjs 6.x * Switch serializer tests to test round trips Rather than separate tests for encoding/decoding. Similar to the other platforms. * update descriptor.proto * Clean up some serializer stuff. 1. The switch to protobufjs 6.x seems to have removed the need for hasTag() (and actually it had broken our usage of it since the tags ended up changing from snake_case to camelCase). So I've removed hasTag() and set `oneofs: false` in protoLoaderOptions so the virtual "fooType" fields are no longer added. 2. 'seconds' in timestamps should always be serialized as a string, since it's an int64 but we weren't doing that. I fixed it, which removed some of the special casing from rsgowman/grpc_load. 3. I reworked the expect...RoundTrip methods into a single unified one which saves some repetition and I think is easier to make sense of. 4. I re-added the disabled useProto3Json assert and instead tweaked the offending test not to try to round-trip through protobufjs. * Update grpc/proto-loader to 0.5.0.
1 parent 46213bb commit be1eb2c

File tree

8 files changed

+1242
-375
lines changed

8 files changed

+1242
-375
lines changed

packages/firestore/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"@firebase/firestore-types": "1.2.1",
3232
"@firebase/logger": "0.1.13",
3333
"@firebase/webchannel-wrapper": "0.2.19",
34+
"@grpc/proto-loader": "^0.5.0",
3435
"grpc": "1.20.0",
3536
"tslib": "1.9.3"
3637
},
@@ -62,6 +63,7 @@
6263
"npm-run-all": "4.1.5",
6364
"nyc": "14.0.0",
6465
"prettier": "1.17.0",
66+
"protobufjs": "6.8.8",
6567
"rollup": "1.10.1",
6668
"rollup-plugin-copy-assets": "1.1.0",
6769
"rollup-plugin-node-resolve": "4.2.3",

packages/firestore/src/platform_node/load_protos.ts

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,64 @@
1515
* limitations under the License.
1616
*/
1717

18+
import * as protoLoader from '@grpc/proto-loader';
1819
import * as grpc from 'grpc';
19-
import { resolve } from 'path';
20+
import * as path from 'path';
21+
import * as ProtobufJS from 'protobufjs';
22+
23+
/** Used by tests so we can match @grpc/proto-loader behavior. */
24+
export const protoLoaderOptions: ProtobufJS.IConversionOptions = {
25+
longs: String,
26+
enums: String,
27+
defaults: true,
28+
oneofs: false
29+
};
2030

2131
/**
2232
* Loads the protocol buffer definitions for Firestore.
2333
*
2434
* @returns The GrpcObject representing our protos.
2535
*/
2636
export function loadProtos(): grpc.GrpcObject {
27-
const options = {
28-
// Beware that converting fields to camel case does not convert the tag
29-
// fields in oneof groups (!!!). This will likely be fixed when we upgrade
30-
// to protobufjs 6.x
31-
convertFieldsToCamelCase: true
32-
};
33-
const root = resolve(
37+
const root = path.resolve(
38+
__dirname,
39+
process.env.FIRESTORE_PROTO_ROOT || '../protos'
40+
);
41+
const firestoreProtoFile = path.join(
42+
root,
43+
'google/firestore/v1/firestore.proto'
44+
);
45+
46+
const packageDefinition = protoLoader.loadSync(firestoreProtoFile, {
47+
...protoLoaderOptions,
48+
includeDirs: [root]
49+
});
50+
51+
return grpc.loadPackageDefinition(packageDefinition);
52+
}
53+
54+
/** Used by tests so we can directly create ProtobufJS proto message objects from JSON protos. */
55+
export function loadRawProtos(): ProtobufJS.Root {
56+
const root = path.resolve(
3457
__dirname,
3558
process.env.FIRESTORE_PROTO_ROOT || '../protos'
3659
);
37-
const firestoreProtoFile = {
60+
const firestoreProtoFile = path.join(
3861
root,
39-
file: 'google/firestore/v1/firestore.proto'
62+
'google/firestore/v1/firestore.proto'
63+
);
64+
65+
const protoRoot = new ProtobufJS.Root();
66+
// Override the resolvePath function to look for protos in the 'root'
67+
// directory.
68+
protoRoot.resolvePath = (origin: string, target: string) => {
69+
if (path.isAbsolute(target)) {
70+
return target;
71+
}
72+
return path.join(root, target);
4073
};
41-
return grpc.load(firestoreProtoFile, /*format=*/ 'proto', options);
74+
75+
protoRoot.loadSync(firestoreProtoFile);
76+
protoRoot.resolveAll();
77+
return protoRoot;
4278
}

packages/firestore/src/protos/firestore_proto_api.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,9 @@ export declare namespace firestoreV1ApiClientInterfaces {
353353
booleanValue?: boolean;
354354
integerValue?: string;
355355
doubleValue?: number;
356-
timestampValue?: string;
356+
timestampValue?: string | { seconds: long; nanos: long };
357357
stringValue?: string;
358-
bytesValue?: string;
358+
bytesValue?: string | Uint8Array;
359359
referenceValue?: string;
360360
geoPointValue?: LatLng;
361361
arrayValue?: ArrayValue;

0 commit comments

Comments
 (0)