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

fix function clone in utils.ts #28

Closed
antonRoyenko opened this issue Aug 11, 2022 · 13 comments · Fixed by #60
Closed

fix function clone in utils.ts #28

antonRoyenko opened this issue Aug 11, 2022 · 13 comments · Fixed by #60

Comments

@antonRoyenko
Copy link

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

@zdev-online
Copy link

Raise the topic

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

@KnorpelSenf
Copy link
Member

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

@antonRoyenko
Copy link
Author

Yes, I could fix it during this week

@KnorpelSenf
Copy link
Member

Thank you so much, looking forward to your changes!

@KnorpelSenf
Copy link
Member

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 :)

@KnorpelSenf
Copy link
Member

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

@KnorpelSenf
Copy link
Member

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

@snussik
Copy link

snussik commented Oct 23, 2022

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.

@KnorpelSenf
Copy link
Member

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.

@snussik
Copy link

snussik commented Oct 24, 2022

@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.

@KnorpelSenf
Copy link
Member

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.

@Madnex
Copy link

Madnex commented Oct 31, 2022

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...

@KnorpelSenf
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants