Hardcode grpc-js version #5771
Hardcode grpc-js version #5771
Conversation
|
Binary Size ReportAffected SDKsTest Logs |
|
|
Size Analysis Report |
It looks like import.meta itself is not the problem, the createRequire is the problem, in that it creates a non-native It seems like createRequire was introduced in Node 12.2.0 while import.meta was introduced in Node 10.4.0. This isn't the direct cause of the problem (the platforms the users are reporting in the issue should be on Node 12 or greater already) but since the Firebase JS SDK supports Node 10+, we probably shouldn't be including an API that requires Node 12. |
| const require = module.createRequire(import.meta.url); | ||
| // eslint-disable-next-line @typescript-eslint/no-require-imports | ||
| const { version: grpcVersion } = require('@grpc/grpc-js/package.json'); | ||
| // TODO: Fetch runtime version from grpc-js/package.json instead |
There was a problem hiding this comment.
IIRC in order to be able to merge this PR, we are waiting on changes from Firestore to surface the GRPC that is why this is not complete? or am I missing something?
There was a problem hiding this comment.
No, this is a stopgap measure so that users can use the SDK without errors for now. In the future if and when the grpc-js team (outside of Firebase) is able to provide us with an export, the plan is we'll revert this and use that instead.
There was a problem hiding this comment.
I see so we are not actually intending on passing any sort of version at the moment, but rather a placeholder string and in some future we will pass along the version. Makes sense, just wasn't clear if this TODO was for now or later, thank you for clarifying
There was a problem hiding this comment.
Oh no, we will pass a version. The version will be grabbed by Rollup at build time based on the version of grpc-js installed when we build the Firebase SDK. Ideally we want the version installed by the user in their node_modules dir, which may differ, but since we specify the version in @firebase/firestore's package.json dependencies, that should be the version they have for the most part.
Line 75 in rollup.config.js above does the string replacement.
Hardcode the grpc-js version used for logging. This isn't ideal, as getting the version at runtime provides the most accurate version for metrics and debugging, but working around problems with import.meta.url in ESM Node builds while ensuring CJS Node builds still work is causing a lot of problems. Make a TODO to change back to getting runtime version when there is a better way to support it across CJS and ESM Node builds.
See issues:
#5752
#5687
And PR:
#5532