ethereumjs / ethereumjs-monorepo

Monorepo for the Ethereum VM TypeScript Implementation
2.6k stars 759 forks source link

Monorepo: Remove most prominent Default Exports #2013

Closed holgerd77 closed 2 years ago

holgerd77 commented 2 years ago

We have decided to remove selected (the most prominent ones) default exports from our libraries, since we are not yet doing an ESM build of our libraries along the v6 breaking release switch and at the same time Node.js/JavaScript/CommonJS users have issues with default exports and we have old issues like https://github.com/ethereumjs/ethereumjs-monorepo/issues/978 where people were complaining about the topic.

Affected imports will then change like this:

import Common from '@ethereumjs/common'

becomes:

import { Common } from '@ethereumjs/common'

I guess the Common export is already the most prevalent one and the one with the most complaints. πŸ˜‹

While a search on export default in the monorepo packages gives ~50 entries most of these are from tests, not exposed on the API level, rarely used functionality, so we do not need to update the majority of this but rather a very selected set with some level of flexibility.

Here is a list of exports I think we should likely update.

For sure (attention: opinionated, feel free to comment if you thing something doesn't make sense)

Would be nice:

(ok, would very much make sense to take just these two would-be-nice candidates in as well πŸ˜‹)

holgerd77 commented 2 years ago

@gabrocheleau πŸ‘

(hehe, just "live-viewed" your assignment πŸ˜‹)

Note that in the VM the current EEI export (accessed by the Beta 1 releases) is also not working properly, I had to use to following weird code adoption from the current EVM README example to get things working:

import Common, { Chain, Hardfork } from '@ethereumjs/common'
import Blockchain from '@ethereumjs/blockchain' 
import { EEI } from '@ethereumjs/vm'
import EVM from '@ethereumjs/evm'
import { DefaultStateManager } from '@ethereumjs/statemanager' 

// Note: in a future release there will be an EEI default implementation
// which will ease standalone initialization
const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.London })
const blockchain = await Blockchain.create({ common })
const stateManager = new DefaultStateManager({ common })
const eei = new EEI.default(stateManager, common, blockchain)

const evm = new EVM({
        common,
        blockchain,
        eei,
      })

This should be checked if this can be moved "back to normal" once default export has been removed (this current * based export * as EEI from './eei/eei' in VM seems generally unnecessary and error prone to me for single-class/item-exports (e.g. here: just EEI and nothing else), think a direct reference without using * should in these cases generally be preferred).