FuelLabs / fuels-ts

Fuel Network Typescript SDK
https://docs.fuel.network/docs/fuels-ts/
Apache License 2.0
44.04k stars 1.35k forks source link

Improve `launchTestNode` integration with `typegen` contract factories #2708

Open nedsalk opened 4 months ago

nedsalk commented 4 months ago

The interface of launchTestNode can be simplified:

import { launchTestNode } from 'fuels/test-utils';
import { CallTestContractAbi__factory } from '../test/typegen/contracts';

const {
    contracts: [contract],
    cleanup,
  } = await launchTestNode({
    contractsConfigs: [ 
      CallTestContractAbi__factory, 
      { factory: CallTestContractAbi__factory, options: { storageSlots: {}, ... } }
    ],
  });

The contractsConfigs can now accept a factory directly

{ contractConfigs: [ CallTestContractAbi__factory ] }

but there can still be flexibility for when users want to use DeployContractOptions via the options property:

{ contractConfigs: [ { factory: CallTestContractAbi__factory, options: { storageSlots: {}, ... } } ] }
danielbate commented 3 months ago

This was closed by #2686.

petertonysmith94 commented 3 months ago

@danielbate are you sure?

I believe this refers to passing in factories as an array element:

contractConfigs: [ CallTestContractAbi__factory ]

Currently we only support an object:

contractConfigs: [{
  factory: CallTestContractAbi__factory
}]
danielbate commented 3 months ago

I closed it as #2686 meant we no longer had to pass bytecode as config.

Personally I like the explicitness of passing the object rather than object OR factory.

nedsalk commented 3 months ago

@danielbate as @petertonysmith94 said, this wasn't completed. Having to pass the bytecode as config was the thing blocking it and #2824 resolved that. I will reopen it.

danielbate commented 3 months ago

IMO there isn't a massive improvement from:

contractConfigs: [{
  factory: CallTestContractAbi__factory
}]

to:

contractConfigs: [ CallTestContractAbi__factory ]

Just seems like we are going to be type overloading if we're still going to offer the flexibility of both, as we still need to offer options as a property. launchTestNode has a lot of config options and I think we benefit from explicit config here.

nedsalk commented 3 months ago

The options property is rarely used even in our own tests - maybe nowhere except launch-test-node.test.ts - so making the default simpler is a win in my book.