RFC Proposal for simplifying how smart contracts are found in a project

[jasongitmail]

trafficstars

Background

When we wrote the CLI initially, we hadn't yet required developers to import & export their smart contracts within index.ts. We added this to allow use of smart contracts that are published to NPM because it's necessary to expose the classes in a common manner.

Because of this, we searched the project to find any class extending SmartContract during deployment. This is easy for developers, but also left one edge case that we need to handle, which is dealing with 2+ smart contract having the same name.

Proposal

During deployment, instead of generating a build.json and using findSmartContracts(), we could instead import the developer's index.ts, stringify class names that it exports, and look at these as the list of available smart contracts that can be deployed. This would enforce uniqueness of all class names and encourage developers to use our newer pattern of importing & exporting smart contracts within their index.ts because doing so would become a requirement for deployment as well.

(If making this change, we can also close this related issue about warning if the same contract appears 2+ in the same project.)

[jasongitmail]

CC @ymekuria wdyt?

[ymekuria]

and encourage developers to use our newer pattern of importing & exporting smart contracts within their index.ts because doing so would become a requirement for deployment as well.

I think this approach will work well @jasongitmail. If we go this route, we probably need to give users more guidance on import export pattern for index.ts. Currently, a user might only start thinking about this pattern when they are ready to integrate a contract into a UI. They will likely use the CLI to deploy before that point.

[mitschabaude]

+1 @jasongitmail, this approach seems cleaner / more explicit, I like how it naturally enforces uniqueness.

There are two ways we could obtain the class name string:

• either we use the string under which the class is exported, i.e. the one from Object.keys(await import('index.js'))
• or we use Class.name which is the name under which the class is declared

These two will only differ in edge cases, say when the developer exports it with something like export {MyContract as MyContractRenamed}. But strictly speaking, only the first of the two ensures uniqueness of the name, and feels more precise to me

[jasongitmail]

Thanks @mitschabaude. +1 for the first of those two because it provides consistency with the name of exports that would be available if the developer's project were published to NPM too.

@ymekuria Good note on the docs. It'd warrant adding a paragraph at the start of the Deploy section, similar to the current import/export paragraph for the "Publish to NPM" section

[mitschabaude]

Something that came up when @carterbrett tried to deploy a zkapp -- error messages should be updated to guide the user to export their smart contract from index.ts. Made an issue here: https://github.com/o1-labs/zkapp-cli/issues/234 I think ensuring a good error message can be part of this RFC's implementation

[jasongitmail]

Agreed. That's in scope here