grammyjs/conversations

fix function clone in utils.ts

antonRoyenko opened this issue · 13 comments

If I had in context property with BigInt, for example user with telegramId I have issue Do not know how to serialize a BigInt in line const string = JSON.stringify(arg);
Screenshot 2022-08-11 at 12 30 08

Raise the topic

This is an important problem that needs to be solved, because we are talking about Telegram ID

For bots, Telegram ID is of type number not bigint. This is an important problem nonetheless, are you willing to look into fixing this?

Yes, I could fix it during this week

Thank you so much, looking forward to your changes!

I have evaluated a variety of different approaches now, including devalue, lave, arson, node-tosource, serialize-javascript, jsesc, superjson, and a few custom implementations, and I have concluded that arson provides the best starting point for creating the solution to our problem. You may be interested in following benjamn/arson#16, but of course nothing is stopping you from simply rolling your own solution :)

I now created a tool which can be used in this plugin. It is able to solve almost every problem we encountered. It is called oson on /x and o-son on npm. The repo is here: https://github.com/KnorpelSenf/oson

@antonRoyenko are you still interested in working on this? :)

The bug is still here. With this structure of object I get the same Do not know how to serialize a BigInt error:

{
  instance: Fluent {
    options: {},
    bundles: Set(2) { [FluentBundle], [FluentBundle] },
    warningHandler: LoggingWarningHandler { options: {}, writeLog: [Function: warn] },
    defaultBundle: FluentBundle {
      _terms: Map(0) {},
      _messages: [Map],
      locales: [Array],
      _functions: [Object],
      _useIsolating: false,
      _transform: [Function: transform],
      _intls: Map(0) {}
    }
  },
  renegotiateLocale: [AsyncFunction: negotiateLocale],
  useLocale: [Function: useLocale]
}

@KnorpelSenf buy the way: the problem in utils can be simply solved by lodash.cloneDeep().
But it seems, that JSON.stringify method is widely used all over other modules and plugins (Redis adapter for ex.). So this error moves deeper.

the problem in utils can be simply solved by lodash.cloneDeep()

This is false. This will not help with bigint support in any way.

So this error moves deeper.

I do not understand what you mean. The problem is already solved with the library I linked above. As soon as we integrate it into this plugin, everything will be fixed.

@KnorpelSenf in my case this fix helped (btw, I store object of User type in my ctx):

export function clone<T>(arg: T) {
 if (arg === undefined) return undefined;
 return _.cloneDeep(arg)
}

I've tried to use your oson lib (for node), but in my case I got empty array [ ].

I do not understand what you mean.

I mean that grammy was developed with assumption that telegram's ids can be just of number type as it's written here. That's why you can meet JSON.stringify(ctx, session and so on) in grammy's code base .

So it doesn't matter that you solve the problem in conversation module. If you have a bigint type in the object, the error will be raised in some other part of code. In order to fix it completely the type should be changed to bigint in @grammyjs/types and all JSON.stringify() occurrences changed to some other method.

In my case I had to change my type for telegram ids from BigInt to number in code and database schema. That helped.

This cloning function can only work with some storage adapters. It is a flawed approach.

Number values are implemented exactly according to the specification. This assumption is valid and it is never going to change.

If you use bigint, then you're doing something that's unrelated to bots. For example, Telegram user bots need this data type. Such values are never going to be processed by grammY.

Supporting bigint values is only a problem when it comes to storing arbitrary data. This is what the conversations plugin does internally. Note that it's the only grammY plugin that needs to store arbitrary data. That's why this is the only plugin that's affected by this issue. Using oson in this plugin will allow us to convert arbitrary data into a format that can be safely passed to JSON.stringify. Thus, the issue will be fixed for all storage adapters.

Is there an update on this? I'm encountering the same issue. What's the best workaround for now? I'm using Prisma + PostgreSQL and BigInt for the telegramID...

As a workaround, you can convert bigints to string and back until oson is integrated into this plugin.