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

Bug: json-bigint-patch causes CASL ability.can() to fail with BigInt conditions #1016

Open
guyhirsch opened this issue Jan 15, 2025 · 6 comments

Comments

@guyhirsch
Copy link

Describe the bug
When importing json-bigint-patch, the ability check in CASL fails to correctly match conditions using BigInt. Specifically, ability.can() incorrectly returns false when it should return true.

To Reproduce
Steps to reproduce the behavior:

  1. Ability configuration:
import 'json-bigint-patch';
import { createMongoAbility, subject } from '@casl/ability';

class Article {
  id: bigint;
}

type Actions = 'all';
type Subjects = Article | typeof Article | 'Article';

const ability = createMongoAbility<[Actions, Subjects]>([
  {
    action: 'all',
    subject: 'Article',
    conditions: { id: 5n },
  },
  {
    action: 'all',
    subject: 'Article',
    conditions: { id: 1n },
  },
]);

const article = new Article();
article.id = 1n;

console.log(ability.can('all', subject('Article', article))); // returns false, but it should return true
  1. How abilities are checked:
    Using ability.can() with a condition that should match, but it fails when json-bigint-patch is imported.

Expected behavior
When json-bigint-patch is imported, ability.can() should return true for matching conditions, just as it does without the import.

CASL Version

@casl/ability - v6.7.3

Environment:

  1. Node.js: v22.12.0
  2. TypeScript: v5.7.2
  3. json-bigint-patch: v0.0.8
@guyhirsch guyhirsch added the bug label Jan 15, 2025
@stalniy
Copy link
Owner

stalniy commented Jan 15, 2025

json-bigint-patch adds custom toJSON method. Ucsst expects toJSON to return primitive value, but in that patch lib it returns non primitive. I think it actually returns BigInt objectified instance. I’ll double check tomorrow

@guyhirsch
Copy link
Author

guyhirsch commented Feb 11, 2025

Hi,
Just following up to check if there’s been any progress or updates on this issue. We’d appreciate any suggestions or information, including any temporary workarounds.
Thanks!

@stalniy
Copy link
Owner

stalniy commented Feb 11, 2025

I've just tested the functionality in ucast/mongo2js and it works with native BigInt. This means it's not a bug but enhancement because you request to make casl work with the patched version of BigInt provided by json-bigint-patch

Then I'm wondering why do you even want to use this patched version. You could use custom serializer + reviver in JSON functions. For example:

const num = BigInt('9223372036854775807')

const serializedBigInt = JSON.stringify(num , (key, value) => {
  return typeof value === 'bigint' ? `BigInt(${value.toString()})` : value
});

const parsedBigInt = JSON.parse(serializedBigInt, (key, value) => {
  return typeof value === 'string' && value.startsWith('BigInt(') ? BigInt(value.slice(7, -1)) : value
});

@stalniy stalniy added enhancement and removed bug labels Feb 11, 2025
@guyhirsch
Copy link
Author

I wanted to share my experience dealing with large numbers in GraphQL and how it relates to this issue. Initially, I followed the recommendation in the GraphQL Scalars BigInt docs to install json-bigint-patch. However, after understanding more about how serialization works, I realized it wasn’t necessary.

Instead, I replaced it with the following approach, which ensures proper serialization without needing the additional library:

(BigInt.prototype as any).toJSON = function () {
  return this.toString();
};

This automatically converts BigInt values to strings during JSON serialization and works seamlessly with the GraphQL server. It simplifies the setup, and so far, I’ve had no issues with large number precision in queries or responses.

I thought this could be a cleaner solution for others facing similar issues. Let me know your thoughts on this approach!

Thanks again for all your work on CASL.

@stalniy
Copy link
Owner

stalniy commented Feb 11, 2025

There is one drawback by embedding this into toJSON because uscast uses toJSON to transform value objects like Mongo ObjectId to primitives. In case of BigInt, you will loose ability to use $lt, $gt & co operators with your current solution. Because BigInts will be compared as strings in this case and not as numbers.

@stalniy
Copy link
Owner

stalniy commented Feb 11, 2025

I will change logic then, instead of relying on duck typing, I will rely on duck typing only if typeof for value === 'object'. In this case, there will no be issues like this

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

No branches or pull requests

2 participants