protocolbuffers / protobuf-javascript

BSD 3-Clause "New" or "Revised" License
375 stars 67 forks source link

BUG: Timestamp.toObject() returning zeroed value #222

Open kyleshepherd opened 2 months ago

kyleshepherd commented 2 months ago

In the latest version, 3.21.4, the Timestamp.toObject() function isn't working correctly.

Previously it would return the timestamp as an object with keys for seconds and milliseconds. However now it is returning zero values for both.

This is a function where I noticed the issue, I've added some logging which pointed out toObject() as being the problem.

function dateToTs(date: Date): Timestamp {
  const ts = new Timestamp();
  ts.fromDate(date);
  console.log(
    "date:",
    date,
    "timestamp:",
    ts.getSeconds(),
    ts.getNanos(),
    ts.toObject(),
  );
  return ts;
}

See the screenshot below for the resulting console log, where you can see the seconds and milliseconds are set, but the toObject return is zeroed image

dibenede commented 2 months ago

I'm having trouble reproducing the issue. I get:

date: 2022-02-01T00:00:00.000Z timestamp: 1643673600 0 { seconds: 1643673600, nanos: 0 }
date: 2022-02-02T00:00:00.000Z timestamp: 1643760000 0 { seconds: 1643760000, nanos: 0 }
date: 2022-02-03T00:00:00.000Z timestamp: 1643846400 0 { seconds: 1643846400, nanos: 0 }

Are you using the google.protobuf.Timestamp proto we ship in the google-protobuf npm package (google-protobuf/google/protobuf/timestamp_pb.js)?

kyleshepherd commented 2 months ago

odd! this is how we're importing the Timestamp

import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb";

and yes we're using the google-protobuf NPM package at version 3.21.4

dibenede commented 2 months ago

I don't think import { Timestamp } from "google-protobuf/google/protobuf/timestamp_pb"; works with plain google-protobuf. Our project doesn't officially support ES modules; just CommonJS.

Could there be any tooling on your side that's transforming the source to a module?

Or maybe building our package from source? I know there's a pending PR that tries to add module support.

My repro attempt was based on CommonJS require's. Trying to use import against an npm install'd google-protobuf errors out.

kyleshepherd commented 2 months ago

I can't see anything in our project that is transforming it, we've been using it as-is for the past 12-18 months without issue up until this 3.21.4 release

dibenede commented 2 months ago

Are you using plain node or ts-node? And what version?

Also, are you using any node flags related module loading? Particularly, experimental ones.

kyleshepherd commented 2 months ago

We're using this in a React app along with Typescript, using Node v20.17.0 and Typescript v5.5.4.

I can't see any node flags anywhere, so I don't believe we are