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

Cache params ordering issue #193

Merged
merged 11 commits into from
Dec 17, 2022

Conversation

Minhir
Copy link
Collaborator

@Minhir Minhir commented Nov 30, 2022

In the first commit, I represented the current problem with cache. Now, query params {a: 1, b: 2} and {b: 2, a: 1} are resolved as different ones.

In the second commit, I solved the problem with fast-json-stable-stringify library as an example. I'm opened to discussion: maybe we will find another solution or hardcode stable-json-hash inside core package.

fixes #156

Copy link
Owner

@igorkamyshev igorkamyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea 💙

However, I want to keep the core package with only one dependency — effector. So, I guess we can reimplement fast-json-stable-stringify inside package.

@igorkamyshev igorkamyshev added type:enhancement New feature or request scope:core labels Dec 5, 2022
@Minhir
Copy link
Collaborator Author

Minhir commented Dec 15, 2022

Will close #156

Copy link
Owner

@igorkamyshev igorkamyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add changeset about fixes bug with un-serializable params? After that, we can ,erge it.

Minhir and others added 2 commits December 17, 2022 00:28
Co-authored-by: Igor Kamyşev <igor@kamyshev.me>
@igorkamyshev igorkamyshev merged commit 41096d5 into igorkamyshev:master Dec 17, 2022
@Minhir Minhir deleted the cache-params-ordering-issue branch December 17, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:core type:enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check if params of the Query is serizlized before key generation
2 participants