BigInteger support #7
Conversation
kartik-gupta-ij
force-pushed
the
bigInt
branch
from
08b5e8e to
2ef41a1
Compare
| "pnpm": { | ||
| "overrides": { | ||
| "@babel/traverse": "7.23.2", | ||
| "semver": "7.5.2", | ||
| "tough-cookie": "4.1.3", | ||
| "word-wrap": "1.2.4", | ||
| "graphql": " 16.8.1 " | ||
| } | ||
| }, |
There was a problem hiding this comment.
Those packages has vulnerabilities
There was a problem hiding this comment.
I would encourage you to explore more modern alternatives. This one was last published 4 years ago, and it seems to have quite a few issues opened.
I found one that might be a better candidate: https://github.com/Ivan-Korolenko/json-with-bigint/blob/main/json-with-bigint.js (much, much smaller and simpler and perhaps even faster?) from this S.O. discussion: https://stackoverflow.com/a/69644630
There was a problem hiding this comment.
@Rendez, The vulnerabilities exist in jest, msw, @typescript-eslint, jest-environment-jsdom, and @changesets/cli, and they have not been resolved yet.
There was a problem hiding this comment.
@Rendez, The vulnerabilities exist in
jest,msw,@typescript-eslint,jest-environment-jsdom, and@changesets/cli, and they have not been resolved yet.
Can't we update these libraries to latest? Do they still have these vulnerabilites after updating?
There was a problem hiding this comment.
Yes, they have vulnerabilities in the latest version too.
|
shouldn't this be also changed https://github.com/qdrant/openapi-typescript-fetch/blob/f33b84870048e75efe68e1ebaf6876bf87548ab1/src/fetcher.ts#L97C2-L97C79 ? |
Fixed |
| "pnpm": { | ||
| "overrides": { | ||
| "@babel/traverse": "7.23.2", | ||
| "semver": "7.5.2", | ||
| "tough-cookie": "4.1.3", | ||
| "word-wrap": "1.2.4", | ||
| "graphql": " 16.8.1 " | ||
| } | ||
| }, |
There was a problem hiding this comment.
I would encourage you to explore more modern alternatives. This one was last published 4 years ago, and it seems to have quite a few issues opened.
I found one that might be a better candidate: https://github.com/Ivan-Korolenko/json-with-bigint/blob/main/json-with-bigint.js (much, much smaller and simpler and perhaps even faster?) from this S.O. discussion: https://stackoverflow.com/a/69644630
tsconfig.base.json
Outdated
| @@ -5,7 +5,7 @@ | |||
| "declarationDir": "dist", | |||
| "esModuleInterop": true, | |||
| "forceConsistentCasingInFileNames": true, | |||
| "lib": ["dom", "dom.iterable", "es2019"], | |||
| "lib": ["dom", "dom.iterable", "es2019", "esnext"], | |||
There was a problem hiding this comment.
Please follow this convention: https://github.com/microsoft/TypeScript/wiki/Node-Target-Mapping
src/jsonParser.ts
Outdated
| @@ -0,0 +1,41 @@ | |||
| export type Json = | |||
There was a problem hiding this comment.
Why did you add:
{
"dependencies": {
"json-with-bigint": "^2.1.0"
}
}And then copied the source here?
There was a problem hiding this comment.
I much rather have the dependency in case new versions with fixes happen, etc.
| @@ -112,14 +112,14 @@ describe('fetch', () => { | |||
| }) | |||
|
|
|||
| it(`POST /accepted`, async () => { | |||
There was a problem hiding this comment.
We should test the conversion and parsing of responses with big int.
#6
Fixed using json-bigint