Closed andreibancioiu closed 5 months ago
These are great improvements that will definitely improve the developer experience – consistency is always great!
Adding an idea for consideration with the goal to expose a clean & intuitive API for developers:
In the proposed state, developers will still have to remember various factory class names because of multiple points of entry for each factory type, increasing cognitive load. To improve things further, an idea is to add a parent class (a layer on top), named TransactionFactory
, to have a single point of entry that exposes further sub-factories via its properties – all with a clean API for developers to work with.
This parent class can also abstract behavior of e.g. Relayed-Transactions, Guardians, etc for further simplicity on the developers end.
Examples:
TransactionFactory.transfers.createEGLDTransfer(...) // .transfers -> factories/transferFactory.ts
TransactionFactory.tokens.issueFungible(...) // .tokens -> factories/tokenFactory.ts
TransactionFactory.contracts.deploy(...) // .contracts -> factories/contractFactory.ts
...
Relaying a transaction could then proof as simple as chaining an optional method withRelayer
:
TransactionFactory
.withRelayer(...) // or withRelayer(V1|V2) (optional)
.contracts
.call(...)
By going this route, we also avoid introducing 'Next*' prefixes which would lead to breaking changes in the future.
The overall idea is to simplify the DX & provide a clean API for developers to work with.
While uniformisation is a good (and needed) thing, using “factory” in a JS library might not be intuitive at all for beginner. I see “factories” more as a “Java” thing than a “JS” one.
Besides the intuitiveness there is a risk to have the library more verbose, that can deter non-multiversx dev to work with if they need to read a lot of documentation before being able to do basic stuff.
Will be handled here: https://github.com/multiversx/mx-sdk-specs
Summary
Following the design of
TokenOperationsFactory
, re-structure the other transaction factories and builders.Current state
There are a number of design inconsistencies among the components responsible with constructing ready-to-sign transactions.
Creating a
TokenTransfersFactory
, then a number of transfer transactions:Creating a
TokenOperationsFactory
, then a number of transactions:Creating a
SmartContract
object, then a number of interaction transactions:Creating a
RelayedTransaction(V1|V2)Builder
, then craft a transaction:References:
Design points
TokenOperationsFactory
, as an example.nonce
when calling the method of the factory). SeeTokenOperationsFactory
for an example.Changes to make
TransferTransactionsFactory
- will not receive aGasEstimator
as a constructor parameter anymore, but a factory config, instead.RelayedTransactionV1Builder
andRelayedTransactionV2Builder
would be merged into aRelayedTransactionsFactory
.SmartContract
- which does behave as a transactions factory (in part) would be extracted to aSmartContractTransactionsFactory
.Progressive roll-out
deprecated
, as the new ones are written and released.Next*
. E.g.NextTransferTransactionsFactory
.Next*
prefix will be dropped.Interaction
component will be handled separately (another proposal, another change set etc.)Feedback
Let us know what you think :pray: