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

prettier format instance with cli itself #55

Merged
merged 15 commits into from
Jun 28, 2024
Merged

Conversation

technophile-04
Copy link
Collaborator

Context :

Previously we were running yarn format to format files on instance created. The down side that approach was if user skips the install step we were not able to do yarn format

The reason we need to format file is mainly because of .template.mjs. If we don't do that people would get warning like this

Summary of this PR :

Instead of relying on prettier which is installed in instance project we kept prettier as main dependency in CLI and format .template.mjs in CLI itself before outputting them to instance project.

  • Removed the prettier fomatting task from cli display
  • Also handles the sorting imports correctly inline with what we have at SE-2
    • To test above thing you could run yarn cli -e subgraph
    • after that cd into instance created and do yarn next:lint there shouldn't be any warnings, especially in ScaffoldEthProvider.tsx since their we are appending import at last but they are properly formatter by cli after template is processed.
  • at 0150ad6 I formatted all solidity foundry files inline with SE-2 hardhat solidity formatting by running forge fmt. Also updated foundry.toml config such that forge fmt is very similar to SE-2 hardhat format styling. The reason I say similar is because there is no one to one mapping from prettier-solidity => foundry formatter there are some slight small difference.

Some future improvements:

Currently in this PR I am using hardhat prettier config to format foundry solidity files, as mentioned earlier its very similar but not same.

So if you select foundry and once the project instance is created, if you cd in {projectsInstance/pacakges/foundry} and run forge fmt you will see a git diff due.

Example :

image

To solve this and also better solution for #42 is:

Instead of notifying the user that he need's to install forge in outromessage, we do an early check if forge is present or not (just after user selects solidity framework) and exit the cli as soon as the forge is not present and foundry was choosen.

Doing above will solve two problems:

  • We can user's computer forge to format the files in foundry package
  • Also we don't need to manually run git submodule packageName Untracked changes foundry #42 commands and instead leverage forge install pacakgeName which is more reliable and fast.

Copy link
Member

@rin-st rin-st left a comment

Choose a reason for hiding this comment

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

Great improvement! Tested and working well!

@technophile-04 technophile-04 marked this pull request as draft June 20, 2024 05:18
@technophile-04 technophile-04 marked this pull request as ready for review June 26, 2024 17:39
@technophile-04
Copy link
Collaborator Author

Just update the changes inline with this discussion : #55 (comment)

To test make sure your format on save is off and paste the following unformatted files in :

copy this in `templates/base/packages/nextjs/app/page.tsx` (it has unsorted imports + unformatted space in const address):
"use client";

import Link from "next/link";
import { useAccount } from "wagmi";
import { BugAntIcon, MagnifyingGlassIcon } from "@heroicons/react/24/outline";
import { Address } from "~~/components/scaffold-eth";
import type { NextPage } from "next";

const Home: NextPage = () => {
             const { address: connectedAddress } = useAccount();

  return (
    <>
      <div className="flex items-center flex-col flex-grow pt-10">
        <div className="px-5">
          <h1 className="text-center">
            <span className="block text-2xl mb-2">Welcome to</span>
            <span className="block text-4xl font-bold">Scaffold-ETH 2</span>
          </h1>
          <div className="flex justify-center items-center space-x-2">
            <p className="my-2 font-medium">Connected Address:</p>
            <Address address={connectedAddress} />
          </div>
          <p className="text-center text-lg">
            Get started by editing{" "}
            <code className="italic bg-base-300 text-base font-bold max-w-full break-words break-all inline-block">
              packages/nextjs/app/page.tsx
            </code>
          </p>
          <p className="text-center text-lg">
            Edit your smart contract{" "}
            <code className="italic bg-base-300 text-base font-bold max-w-full break-words break-all inline-block">
              YourContract.sol
            </code>{" "}
            in{" "}
            <code className="italic bg-base-300 text-base font-bold max-w-full break-words break-all inline-block">
              packages/hardhat/contracts
            </code>
          </p>
        </div>

        <div className="flex-grow bg-base-300 w-full mt-16 px-8 py-12">
          <div className="flex justify-center items-center gap-12 flex-col sm:flex-row">
            <div className="flex flex-col bg-base-100 px-10 py-10 text-center items-center max-w-xs rounded-3xl">
              <BugAntIcon className="h-8 w-8 fill-secondary" />
              <p>
                Tinker with your smart contract using the{" "}
                <Link href="/debug" passHref className="link">
                  Debug Contracts
                </Link>{" "}
                tab.
              </p>
            </div>
            <div className="flex flex-col bg-base-100 px-10 py-10 text-center items-center max-w-xs rounded-3xl">
              <MagnifyingGlassIcon className="h-8 w-8 fill-secondary" />
              <p>
                Explore your local transactions with the{" "}
                <Link href="/blockexplorer" passHref className="link">
                  Block Explorer
                </Link>{" "}
                tab.
              </p>
            </div>
          </div>
        </div>
      </div>
    </>
  );
};

export default Home;
Copy this in hardhat template YourContract.sol (unformatted solidity code)
//SPDX-License-Identifier: MIT
pragma solidity >=0.8.0 <0.9.0;

// Useful for debugging. Remove when deploying to a live network.
                     import "hardhat/console.sol";

// Use openzeppelin to inherit battle-tested implementations (ERC20, ERC721, etc)
// import "@openzeppelin/contracts/access/Ownable.sol";

/**
 * A smart contract that allows changing a state variable of the contract and tracking the changes
 * It also allows the owner to withdraw the Ether in the contract
 * @author BuidlGuidl
 */
contract YourContract {
	// State Variables
	       address public immutable owner;
	string public greeting = "Building Unstoppable Apps!!!";
	                  bool public premium = false;
	uint256 public totalCounter = 0;
	                     mapping(address => uint) public userGreetingCounter;

	// Events: a way to emit log statements from smart contract that can be listened to by external parties
	event GreetingChange(address indexed greetingSetter,string newGreeting,
		bool premium,
		uint256 value
	);

	// Constructor: Called once on contract deployment
	// Check packages/hardhat/deploy/00_deploy_your_contract.ts
	constructor(address _owner) {owner = _owner;
	}

	// Modifier: used to define a set of rules that must be met before or after a function is executed
	// Check the withdraw() function
	modifier isOwner() {
		// msg.sender: predefined variable that represents address of the account that called the current function
		require(msg.sender == owner, "Not the Owner");
		_;
	}

	/**
	 * Function that allows anyone to change the state variable "greeting" of the contract and increase the counters
	 *
	 * @param _newGreeting (string memory) - new greeting to save on the contract
	 */
	function setGreeting(string memory _newGreeting) public payable {
		// Print data to the hardhat chain console. Remove when deploying to a live network.
		console.log(
			"Setting new greeting '%s' from %s",
			_newGreeting,
			msg.sender
		);

		// Change state variables
		greeting = _newGreeting;
		totalCounter += 1;
		userGreetingCounter[msg.sender] += 1;

		// msg.value: built-in global variable that represents the amount of ether sent with the transaction
		if (msg.value > 0) {
			premium = true;
		} else {
			premium = false;
		}

		// emit: keyword used to trigger an event
		emit GreetingChange(msg.sender, _newGreeting, msg.value > 0, msg.value);
	}

	/**
	 * Function that allows the owner to withdraw all the Ether in the contract
	 * The function can only be called by the owner of the contract as defined by the isOwner modifier
	 */
	function withdraw() public isOwner {
		(bool success, ) = owner.call{ value: address(this).balance }("");
		require(success, "Failed to send Ether");
	}

	/**
	 * Function that allows the contract to receive ETH
	 */
	receive() external payable {}
}

Now run :

yarn cli ../test-hardhat --skip-install -s hardhat

Look at ../test-hardhat page.tsx and YourContract.sol they both should formatted

To test Foundry simply run :

yarn cli ../test-foundry --skip-install -s foundry

And then run :

cd ../test-foundry && forge fmt --root packages/foundry

then do git status it won't give you any changes since everything was perfectly formatted, If we didn't formatting working properly it would have give changes in Deploy.s.sol since that's a template file and leaves an extra line in final version.

@rin-st
Copy link
Member

rin-st commented Jun 26, 2024

GJ! Foundry works well, but getting this error for hardhat.

Details

yarn cli ../test-hardhat --skip-install -s hardhat

 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 | Create Scaffold-ETH 2 app |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+



✔ 📁 Create project directory /Users/liana/Documents/prog/buidl-guidl/test-hardhat
✔ 🚀 Creating a new Scaffold-ETH 2 app in ../test-hardhat
✖ Failed to run prettier
◼ 📦 Installing dependencies with yarn, this could take a while
◼ 📡 Initializing Git repository
ERROR Error occurred Error: Failed to run prettier
    at prettierFormat (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/dist/cli.js:505:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Task.run (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/node_modules/listr2/dist/index.js:2056:11) {
  [cause]: Error: Command failed with exit code 1: yarn prettier --write /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/deploy/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/scripts/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/test/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/contracts/**/*.sol --config /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/.prettierrc.json --plugin=prettier-plugin-solidity
  [error] No files matching the pattern were found: "/Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/deploy/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/scripts/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/test/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/contracts/**/*.sol".
      at makeError (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/node_modules/execa/lib/error.js:59:11)
      at handlePromise (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/node_modules/execa/index.js:124:26)
      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async runPrettier (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/dist/cli.js:467:20)
      at async prettierFormat (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/dist/cli.js:494:13)
      at async Task.run (file:///Users/liana/Documents/prog/buidl-guidl/create-eth/node_modules/listr2/dist/index.js:2056:11) {
    shortMessage: 'Command failed with exit code 1: yarn prettier --write /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/deploy/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/scripts/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/test/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/contracts/**/*.sol --config /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/.prettierrc.json --plugin=prettier-plugin-solidity',
    command: 'yarn prettier --write /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/deploy/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/scripts/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/test/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/contracts/**/*.sol --config /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/.prettierrc.json --plugin=prettier-plugin-solidity',
    escapedCommand: 'yarn prettier --write "/Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/deploy/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/scripts/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/test/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/contracts/**/*.sol" --config "/Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/.prettierrc.json" "--plugin=prettier-plugin-solidity"',
    exitCode: 1,
    signal: undefined,
    signalDescription: undefined,
    stdout: '',
    stderr: '[error] No files matching the pattern were found: "/Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/deploy/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/scripts/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/test/**/*.ts /Users/liana/Documents/prog/buidl-guidl/test-hardhat/packages/hardhat/contracts/**/*.sol".',
    failed: true,
    timedOut: false,
    isCanceled: false,
    killed: false
  }
}
Uh oh! 😕 Sorry about that! Exiting...

Looks like because of this change fd3b4be

@technophile-04
Copy link
Collaborator Author

Looks like because of this change fd3b4be

Ohh thanks Rinat! Fixed at 4e982ae


Btw I noticed a small bug too if you do

yarn cli ../test-hardhat --skip-install -s hardhat

And then cd ../test-hardhat && yarn install

After this if you run yarn format, notice there is some printWidth changes in TupleArray.jsx and contract.ts file, still trying to figure out what's the cause of it

@technophile-04
Copy link
Collaborator Author

After this if you run yarn format, notice there is some printWidth changes in TupleArray.jsx and contract.ts file, still trying to figure out what's the cause of it

Umm not sure how to tackle this issue nicely :( seems like the difference because of all eslint prettier thing?

Seems like we get different formatting in SE-2 (where we are using eslint & prettier at same time)

Even tried tinkering with https://github.com/prettier/prettier-eslint-cli#readme but no good luck since its not allowing to pass plugins :( also not able to get the above thing solved as well

The different formatting seems like because of prettier/prettier-eslint#182

@rin-st
Copy link
Member

rin-st commented Jun 27, 2024

Problem is that prettier version inside base/packages/nextjs is ~2.8.4, but at top level of create-eth is 3.2.5. Changing first to 3.2.5 too fixes that bug

Btw, maybe change it to latest ~3.3.2?

upd. It adds warnings for import-order, checking it

@rin-st
Copy link
Member

rin-st commented Jun 27, 2024

adding plugin explicitly to prettierrc.json fixed warning. Also changed prettier to 3.3.2. Formatting and sorting imports works as expected for me

61f35b4

@technophile-04
Copy link
Collaborator Author

Problem is that prettier version inside base/packages/nextjs is ~2.8.4, but at top level of create-eth is 3.2.5. Changing first to 3.2.5 too fixes that bug
adding plugin explicitly to prettierrc.json fixed warning. Also changed prettier to 3.3.2. Formatting and sorting imports works as expected for me

TYSM Rinat 🙏!!! Works nicely!! As I understand CLI was formatting correctly but since SE-2 prettier was bit lower it slightly formatted the code wrongly

Will create an PR at SE-2 to updating prettier version and then we can backmerge those changes to CLI and this branch too 🙌

@technophile-04
Copy link
Collaborator Author

Just tested out everything and all seems to work as expected, Merging this! Thanks all!

@technophile-04 technophile-04 merged commit 4db51ac into main Jun 28, 2024
1 check passed
@technophile-04 technophile-04 deleted the prettier-formatting branch June 28, 2024 15:20
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.

3 participants