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

yarn flatten #745

Merged
merged 3 commits into from
Feb 29, 2024
Merged

yarn flatten #745

merged 3 commits into from
Feb 29, 2024

Conversation

carletex
Copy link
Member

yarn flatten: Allow to flatten contracts into a single file.

I think we could do the same in Foundry: https://book.getfoundry.sh/reference/forge/forge-flatten

Open to suggestions!

@technophile-04
Copy link
Collaborator

technophile-04 commented Feb 28, 2024

Thanks Carlos 🙌

Screen.Recording.2024-02-28.at.9.44.56.PM.mov

For some reason when I run just yarn flatten , the FlattendContracts.sol only contains output till max line 258 rest output is eaten up.

When I cd package/hardhat and manually run yarn hardhat flatten ... it works just fine, not sure if its just with my system.

If the contract is small (eg. I removed import console.log from YourContract.sol) It works nicely not sure if its due some limit on no. of line redirect (">") into file when running from package.json script.

cc @Pabl0cks could you please give it a try, it might be something to do with my machine

@carletex
Copy link
Member Author

Oh weird! @technophile-04

What happens if you remove the redirect in the hardhat package.json? Do you get the full output in the console?

@technophile-04
Copy link
Collaborator

technophile-04 commented Feb 28, 2024

if you remove the redirect in the hardhat package.json? Do you get the full output in the console?

yeah I do get full output in console if I remove redirect from package.json script

@carletex
Copy link
Member Author

carletex commented Feb 28, 2024

no clue what might be 🤷‍♂️

yeah I do get full output in console if I remove redirect from package.json script

With this removed in the package.json, does this work? yarn flatten > test.sol

@technophile-04
Copy link
Collaborator

yarn flatten > test.sol

Yup it creates test.sol at root of monorepo but works nicely !

Also if I put this in root package.json "flatten": "yarn workspace @se-2/hardhat flatten > test.sol", it again breaks, so seems like an issue with redirect.

Does it properly work for you? Lets maybe work for @Pabl0cks to test, I will also try to debug till then 🙌

@carletex
Copy link
Member Author

it again breaks, so seems like an issue with redirect.

I guess it's an issue with redirect inside the package.json, since the redirection works fine in your system (yarn flatten > test.sol)

Does it properly work for you? Lets maybe work for @Pabl0cks to test, I will also try to debug till then 🙌

Yep, it works for me as it's.

AN alternative could be to skip the redirection and just print out on stdout... and people can redirect themselves.

@Pabl0cks
Copy link
Collaborator

Pabl0cks commented Feb 28, 2024

GJ Carlos and Shiv!!

Does it properly work for you? Lets maybe work for @Pabl0cks to test, I will also try to debug till then 🙌

It's working fine to me, I get the same output with yarn flatten and with `yarn hardhat flatten > flattened.sol´ => 1648 lines and seems complete

Copy link
Collaborator

@Pabl0cks Pabl0cks left a comment

Choose a reason for hiding this comment

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

Is working great to me, flattening all direct imports and imports into those imports

Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

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

I guess it's an issue with redirect inside the package.json

💯💯

Hmm after some digging turns out something to do with yarn version (might be how I have globally configured yarn)

When I do yarn set version 1.22.19 It works nicely for me but for some reason it sets that version at project level :(

Demo Video
Screen.Recording.2024-02-29.at.11.52.41.AM.mov

AN alternative could be to skip the redirection and just print out on stdout

+1 for this, I think people can redirect themselves.... just to be safe, eg Autin intuitively doing here

But please free to merge as it ! Tysm Carlos, and Pablo for test 🙌

Will create a PR adding this into foundry 🙌

@carletex
Copy link
Member Author

eg Autin intuitively doing here

Autocompletion for SE-1 haha where it was printed in stdout

Decided to remove the redirect in the script, in case someone else faces the same issue as Shiv

@technophile-04
Copy link
Collaborator

Tysm merge this 🙌

@technophile-04 technophile-04 merged commit 8164270 into main Feb 29, 2024
1 check passed
@technophile-04 technophile-04 deleted the flatten branch February 29, 2024 14:07
@github-actions github-actions bot mentioned this pull request Mar 11, 2024
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

3 participants