Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

[GraphQL]: Optimize cache-key function #2405

Closed
buffalojoec opened this issue Apr 2, 2024 · 12 comments · Fixed by #2482
Closed

[GraphQL]: Optimize cache-key function #2405

buffalojoec opened this issue Apr 2, 2024 · 12 comments · Fixed by #2482
Labels
enhancement New feature or request GraphQL

Comments

@buffalojoec
Copy link
Contributor

buffalojoec commented Apr 2, 2024

Problem

Since fast-stable-stringify doesn't support bigint natively, we've had to use json-stable-stringify in the GraphQL library.

However, json-stable-stringify, is huge, and we shouldn't introduce this bloat to the codebase.

Proposed Solution

We can do one of three things:

  1. Find a very lightweight cache-key function library that supports bigint.
  2. Fork fast-stable-stringify and add bigint support.
  3. PR bigint support into fast-stable-stringify.
@buffalojoec buffalojoec added enhancement New feature or request GraphQL labels Apr 2, 2024
@buffalojoec buffalojoec added this to the GraphQL milestone Apr 2, 2024
@Hrushi20
Copy link
Contributor

Hrushi20 commented Apr 11, 2024

Hey! I'm interested in working on this issue. Do I replace json-stable-stringify with fast-stable-stringify in package.json file and update the code usage?

@buffalojoec
Copy link
Contributor Author

Hey! I'm interested in working on this issue. Do I replace json-stable-stringify with fast-stable-stringify in package.json file and update the code usage?

Hey, I've added some more context to the issue! Let me know if you still have questions.

@Hrushi20
Copy link
Contributor

What is the definition of cache-key function? Also I see that stringify in json-stable-stringify converts the object into JSONString.

@Hrushi20
Copy link
Contributor

We can try option 3, fast-stable-stringify has not been updated since 7years. Not sure if the project is active. We can fallback to option 2 as our next option.

@Hrushi20
Copy link
Contributor

Hrushi20 commented Apr 11, 2024

json-stringify-deterministic also looks like a very good option. There is more than 50% reduction in size of dependency. We need to replace json-stable-stringify in package.json and continue to use the same replacer function in loader.ts file

NameUnpacked Size
json-stable-stringify27.7 kB
json-stringify-deterministic11.6 kB

@buffalojoec
Copy link
Contributor Author

Yeah that's not bad. I'm a bit biased, though because fast-stable-stringify is < 1 kB minified.
https://bundlephobia.com/package/[email protected]

I think if we forked it into our monorepo here and just added bigint support we could still capture ~1 kB minified size, which is awesome, especially for anywhere else we use it.

What do you think of going the fork route? @solana/fast-stable-stringify?

@Hrushi20
Copy link
Contributor

Sounds good to me. I'm working on it.

@Hrushi20
Copy link
Contributor

Hrushi20 commented Apr 12, 2024

Can the forked package be in javascript?

@Hrushi20
Copy link
Contributor

I've forked and updated the package to support BigInt. I've also added some tests to ensure correct behavior.

@buffalojoec
Copy link
Contributor Author

Can the forked package be in javascript?

Yes!

I've forked and updated the package to support BigInt. I've also added some tests to ensure correct behavior.

Are you planning to open a pull request to add it to this monorepo? That would be the ideal place for us!

@Hrushi20
Copy link
Contributor

Can the forked package be in javascript?

Yes!

I've forked and updated the package to support BigInt. I've also added some tests to ensure correct behavior.

Are you planning to open a pull request to add it to this monorepo? That would be the ideal place for us!

Working on adding it to monorepo.

Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 14, 2024
Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 14, 2024
Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 15, 2024
Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 15, 2024
Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 15, 2024
@buffalojoec buffalojoec linked a pull request Apr 15, 2024 that will close this issue
Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 16, 2024
buffalojoec pushed a commit that referenced this issue Apr 16, 2024
* #2405: fast-stable-stringify init

* #2405: Fix tsconfig.json prettier, update changeset file

* #2405: update fast-stable-stringify package.json

* #2405: update README.md

* #2405: update LICENSE

* #2405: address review comments
Hrushi20 added a commit to Hrushi20/solana-web3.js that referenced this issue Apr 17, 2024
Copy link
Contributor

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request GraphQL
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants