Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

Could not parse fraction #20

Open
cl510804194 opened this issue Jul 16, 2022 · 26 comments
Open

Could not parse fraction #20

cl510804194 opened this issue Jul 16, 2022 · 26 comments

Comments

@cl510804194
Copy link

when i copy the code
image
and throw a error such as
image

this is my params
i find source code params new Percent(50, 10_000),also throw the same error

@cl510804194
Copy link
Author

this is my params
image
or
Uploading image.png…

@amanmehra262001
Copy link

Is this issue resolved yet.

@amanmehra262001
Copy link

Switching JSBI version to "jsbi": "^3.1.4", worked for me

@Florian-S-A-W
Copy link

Hi @cl510804194 @amanmehra262001

I could not reproduce your issue with neither the latest version of the smart-order-router package nor the core-sdk package. I can see that the core-sdk has been using "jsbi": "^3.1.4" for quite some time now, what version of what package are you using where you are experiencing this issue?

@ai-blib
Copy link

ai-blib commented Apr 28, 2023

image

Through debugging, I found that the type of ZERO in the code has changed。 Cause the following judgment to fail ![image](https://user-images.githubusercontent.com/76576519/235186511-98002ecd-430f-4e2f-88e7-f4b643e9ed24.png)。 I tried to change the packaging tool of the project to webpack to solve the problem. I used vite before

@ai-blib
Copy link

ai-blib commented Apr 28, 2023

image
It is here that the type judgment fails. You can also modify this code to solve

@Florian-S-A-W
Copy link

Hi @ai-blib

This code works for me without any issues. What packages/versions are you using? Where is this ZERO from?

@ai-blib
Copy link

ai-blib commented Apr 30, 2023

image

image

From this package。 The type of ZARO should be JSBI,In unknown circumstances, the type of ZERO becomes array. Therefore, it is judged that the verification did not pass

image

@ai-blib
Copy link

ai-blib commented Apr 30, 2023

image

@ai-blib
Copy link

ai-blib commented Apr 30, 2023

My ultimate solution is to modify @ uniswap/sdk core.
image.
Until now, I haven't figured out why the type of SERO is sometimes JSBI, and sometimes Array I found through the debugger that when an error is reported, the ZAERO type changes to array. I feel like it's a waste of time, find a reason. So, @ uniswap/sdk core was modified

@Florian-S-A-W
Copy link

Thanks @ai-blib

Unfortunately, JSBI is an extension of Array, so instanceof Array will always be true for any JSBI instance as the instanceof operator checks if the prototype property of a constructor appears anywhere in the prototype chain of the object.
Adding Array as a supported type to construct a Fraction is a hack and would lead to undefined behaviour for any Array that is not a JSBI instance.

So unfortunately we have to find out why ZERO is not a JSBI instance (but still an Array?) in your case.
Are you using the latest version of the router-sdk? Do you have different versions of any of the other sdks installed somehow? Could it be that you are using a different version of JSBI somewhere else in your project?

Ultimately I think it would make more sense to drop JSBI than try to find a solution for this.

@ai-blib
Copy link

ai-blib commented Apr 30, 2023

I also thought it was a version issue at first. router-sdk I will always be the latest version. still have problem。
I also did experiments. create-react-app creates a react project,It is possible to use the latest version,Its packaging tool is webpack。Then, based on vite's new packaging tool, And then my new packaging tool based on vite will report this,Could not parse fraction。I do not know why it has to be like this。
I have tried using the latest version

@Florian-S-A-W
Copy link

Could you send me your OS, your node version and a code snippet that reproduces this issue? I'll try to reproduce it in Docker.

@ai-blib
Copy link

ai-blib commented Apr 30, 2023

But I've been a bit busy lately, wait for me to finish the work at hand. I will send out the project of my experiment, and it took me a lot of time to find the reason for this problem

@Florian-S-A-W
Copy link

Sure, thanks for your help.

@torhector2
Copy link

Hi @Florian-S-A-W

We also find this issue on our project when integrating the Uniswap widget (that uses the sdk-core).
We're using Vite, I tried integrating it with Nextjs and found no issues. My guess is that Vite has some kind of issue with JSBI.

@torhector2
Copy link

We opened an issue in the widget repo, but the source error seems to be the same as here, the tryParseFraction Uniswap/widgets#586

@Florian-S-A-W
Copy link

Thank you @torhector2 that helps a lot. I will look into it.

@rafinskipg
Copy link

did you see any reason why it fails @Florian-S-A-W ?

@rafinskipg
Copy link

image

@ai-blib
Copy link

ai-blib commented May 22, 2023

Hi @Florian-S-A-W

We also find this issue on our project when integrating the Uniswap widget (that uses the sdk-core). We're using Vite, I tried integrating it with Nextjs and found no issues. My guess is that Vite has some kind of issue with JSBI.

Regarding vite, you can try configuring it like this to see if it is feasible

image

@b-pmcg
Copy link

b-pmcg commented May 23, 2023

Hi @Florian-S-A-W
We also find this issue on our project when integrating the Uniswap widget (that uses the sdk-core). We're using Vite, I tried integrating it with Nextjs and found no issues. My guess is that Vite has some kind of issue with JSBI.

Regarding vite, you can try configuring it like this to see if it is feasible

image

This worked for me, but I it uncovered a new error require is not defined. Adding this config option into to the vite config fixes it, but I'm not sure if there's any drawbacks to adding it.

build: {
    commonjsOptions: {
      transformMixedEsModules: true
    },
  },

Also I had to pin jsbi to version 3.x, as there were some breaking changes in v4

@Florian-S-A-W
Copy link

This issue seems to come from Vite rebasing direct node module imports which leads to double instantiating the same dependency and instanceof failing.
See vitejs/vite#9828 and vitejs/vite#9528
It seems that configuring vite like @ai-blib and @b-pmcg works but I don't see an acceptable way of fixing this issue directly in the sdk-core. Not using instanceof would just be incorrect.

melonges added a commit to polus-dev/pay-form that referenced this issue May 27, 2023
@ashutosh887
Copy link

I would like to work on this @cl510804194 @amanmehra262001 @moodysalem

@mrosendin
Copy link

mrosendin commented Dec 5, 2023

Hi @Florian-S-A-W, I had the same issue as @ai-blib using the latest versions of sdk-core and v3-sdk. Changing JSBI version to 3.2.5 did not solve for me. I independently came up with the same solution as @ai-blib. However, I agree with you this is a hacky solution.

I encountered the Could not parse fraction error when running v3-sdk test suite, occurring from Fraction.tryParseFunction in sdk-core. For example, the transpiled sdk-core package didn't seem to register the JSBI type signature for other when invoking Fraction.lessThan(other).

@Florian-S-A-W
Copy link

Florian-S-A-W commented Dec 5, 2023

Hi @mrosendin ,
We created a major update for the v3-sdk and sdk-core for the Uniswap Foundation that removes internal usage of JSBI. It should (hopefully) be merged soon. Just waiting on Uniswap Labs for some reviews.

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

No branches or pull requests

9 participants