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

save compiled contracts to localStorage #241

Merged
merged 3 commits into from
Feb 29, 2024

Conversation

kwkr
Copy link
Contributor

@kwkr kwkr commented Feb 22, 2024

Add saving compiled contract to be able to resume a development session. #178

Copy link

vercel bot commented Feb 22, 2024

@kwkr is attempting to deploy a commit to the Starknet Remix Plugin Team on Vercel.

A member of the Team first needs to authorize it.

@kwkr kwkr marked this pull request as ready for review February 22, 2024 21:08
@kwkr
Copy link
Contributor Author

kwkr commented Feb 22, 2024

A few things that I have noticed (I'm just a beginner when it comes to StarkNet so this can also be my lack of knowledge):

  • when deploying a contract, it's possible to deploy it multiple times which causes deployedInfo to be populated with duplicated data.
  • the form is not allowing/makes it hard to deploy contracts without a constructor. There is always a field value_ that causes the deployment to fail. Is this expected behavior?

@rjnrohit
Copy link
Contributor

A few things that I have noticed (I'm just a beginner when it comes to StarkNet so this can also be my lack of knowledge):

  • when deploying a contract, it's possible to deploy it multiple times which causes deployedInfo to be populated with duplicated data.
  • the form is not allowing/makes it hard to deploy contracts without a constructor. There is always a field value_ that causes the deployment to fail. Is this expected behavior?

Are you talking about this :

which is causing duplication? If yes then yeah, there is a duplication there, you can modify the code to store unique pairs

I just deployed this contract: https://codeshare.io/r49mXY and I'm able to deploy it successfully , if you are getting some error for your contract then please paste the errors here

@kwkr
Copy link
Contributor Author

kwkr commented Feb 26, 2024

@rjnrohit Added the deduplication logic.
When it comes to the problem with the constructor, it seems like the form is not updating correctly. After deploying the contracts, when I select some new contract, the constructor field stays there. See the screenshots.

1

Before deploying the first contract with constructor:
2

After deploying it:
3

@rjnrohit
Copy link
Contributor

I see the problem, would you able to add a fix for this ? Thanks for pointing it out @kwkr

@kwkr
Copy link
Contributor Author

kwkr commented Feb 26, 2024

@rjnrohit , I think the problem lies within the library that is used for rendering the constructor starknet-abi-form. It seems like the form state is somehow hidden in a global state and once it's activated, it couldn't reset it from the plugin level. There seems to be some jotai atom there that is holding the value (there is some field with generic name constructor. I tried different ways to change the state by passing different abi arrays (to refresh the props) but it doesn't work. So I assume the problem is somewhere there.

@rjnrohit
Copy link
Contributor

rjnrohit commented Feb 27, 2024

@kwkr If you want then you contribute to this repo:https://github.com/NethermindEth/starknet-abi-form as well , you can submit a PR there !

@kwkr
Copy link
Contributor Author

kwkr commented Feb 27, 2024

@rjnrohit , okay, will do it in a separate issue then since it doesn't seem to be directly connected to this one.

@kwkr
Copy link
Contributor Author

kwkr commented Feb 28, 2024

@rjnrohit is there anything else that needs to be done for this PR?

@rjnrohit
Copy link
Contributor

Did you check the form behaviour ?

@kwkr
Copy link
Contributor Author

kwkr commented Feb 28, 2024

@rjnrohit The form behaviour is independent of the changes in this PR. It currently exists in the official plugin version available through remix ide marketplace directly. To fix this, the starknet-abi-from would need to be released and then bumped here.

@rjnrohit
Copy link
Contributor

New version of starknet-abi-form has been published, would you able to check it now?

@kwkr
Copy link
Contributor Author

kwkr commented Feb 28, 2024

Thanks @rjnrohit ! I just pushed the update that prevents constructors from becoming stale.

@rjnrohit
Copy link
Contributor

Congratulations, @kwkr! Your PR successfully made it through. 🎉🙏 Keep up the great work and continue contributing! Your efforts are truly valued

@rjnrohit rjnrohit merged commit da5a9e3 into NethermindEth:develop Feb 29, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants