Fuel merge part 8 - It runs 馃帀 - #186
Conversation
DZakh
commented
Sep 9, 2024
DZakh
commented
0ce70b2 to
4db591a
Compare
|
|
||
| format!( | ||
| "let abi = %raw(`null`)\n{}\n{}", | ||
| "let abi = Fuel.transpileAbi(%raw(`require(\"../../{}\")`))\n{}\n{}", |
There was a problem hiding this comment.
I'm always nervous to see hardcoded relative paths. Should this not take some sort of parameter that's been parsed. Like generated relative path?
There was a problem hiding this comment.
Let's fix it when we have a case when it breaks.
There was a problem hiding this comment.
I'm pretty sure even our integration tests would have this case.
There was a problem hiding this comment.
This is how indexer-fuel works. I'll cut the corner here, to make it work and then fix some of the issues. I already have 4 similar things on my list
| .to_rescript_schema(), | ||
| "S.object(s => {fieldA: s.field(\"fieldA\", S.int), fieldB: s.field(\"fieldB\", \ | ||
| .to_rescript_schema(&"eventArgs".to_string()), | ||
| "S.object((s): eventArgs => {fieldA: s.field(\"fieldA\", S.int), fieldB: s.field(\"fieldB\", \ |
There was a problem hiding this comment.
What is the error you get if you don't have the type annotation? Does rescript not infer the type if it's in the same scope?
There was a problem hiding this comment.
There are cases when there are multiple types with the same shape, and ReScript doesn't know which one to use
| {{#if is_evm_ecosystem}} {{!-- TODO: Add validation for Fuel --}} | ||
| // Even though it's the Address.t type on ReScript side, for TS side it's a string. | ||
| // So we need to ensure that it's a valid checksummed address. | ||
| let contractAddress = contractAddress->Address.Evm.fromAddressOrThrow | ||
|
|
||
| {{/if}} |
There was a problem hiding this comment.
Will you add a similar sanitisation function for fuel
There was a problem hiding this comment.
Yes, I'll add it in the next PR
b738c04 to
59a13ec
Compare
|
Thanks for the review. I've fixed the code comments, and I'll do other fixes in the next PR |
JonoPrest
left a comment
JonoPrest
left a comment
There was a problem hiding this comment.
Cool, looks good Dmitry although I don't think the paths are handled properly for referencing the abi file. I would prefer we handle it rather than wait for it to come up.
2 participants
