Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Review of behaviour: exporting imports #193

Closed
axic opened this issue Jan 18, 2016 · 6 comments
Closed

Review of behaviour: exporting imports #193

axic opened this issue Jan 18, 2016 · 6 comments

Comments

@axic
Copy link
Member

axic commented Jan 18, 2016

The majority of the libs are using the exported imports of ethereumjs-util, such as BN, rlp and secp256k1.

I understand the motivation behind that would be code size when processed via browserify. Is that really a problem? Shouldn't browserify work out it as long as the modules in the dependency chain use the same version of the imports?

It is more maintenance work to keep them aligned, but has a less hack-ish feeling.

Additionally, ethereumjs-vm has another set of exports:

module.exports = VM

VM.deps = {
  ethUtil: require('ethereumjs-util'),
  Account: require('ethereumjs-account'),
  Trie: require('merkle-patricia-tree'),
  rlp: require('ethereumjs-util').rlp
}

And ethereumjs-tx too:

// give browser access to Buffers
global.Buffer = Buffer
global.ethUtil = ethUtil

Update: the additions in the VM and the tx library have been removed over time.

@axic axic changed the title Exporting imports Review of behaviour: exporting imports Jan 18, 2016
@axic
Copy link
Member Author

axic commented Jan 19, 2016

cc @wanderer @kumavis

@kumavis
Copy link
Member

kumavis commented Jan 19, 2016

Here are my assumptions:

  • we're only exporting what we're using internally, nothing extra added on
  • the types exported are guaranteed to behave the way we expect to inject
  • different versions of the modules exported should still work, within reason
  • we're mostly doing this for people who havent figured out how empowering an automated dependency graph is -- its for producing bundles that such users can copy-paste in - and not having to say "oh and also go get a bundle for X and Y so you can use this properly"

Thats my thoughts. Not sure what change you are suggesting. Using the exported modules is optional as it works with your own versions, so I see no problem here. Though, do clarify your concerns.

@wanderer
Copy link
Member

I understand the motivation behind that would be code size when processed via browserify. Is that really a problem?

No this is not a problem

Shouldn't browserify work out it as long as the modules in the dependency chain use the same version of the imports?

yep!

There are two reason for the current behavior

  1. as @kumavis to help some front-end devs that didn't use npm
  2. it make keeping all of the common ethereumjs-* deps versions in sync, easy.

@holgerd77
Copy link
Member

holgerd77 commented Nov 6, 2018

Will transfer this to organization (will delete ideas since this never caught off and it is already challenging to maintain one organizational repo).

@holgerd77 holgerd77 transferred this issue from another repository Nov 6, 2018
@holgerd77
Copy link
Member

This is reduced to the question around the ethereumjs-util re-exports, will move the issue there.

@holgerd77 holgerd77 transferred this issue from ethereumjs/organization Apr 30, 2019
@holgerd77
Copy link
Member

I think we have found a good balance here with dropping the cryptosecp256k1 reexport with v7 in #249 and keeping the BN.js and rlp reexports. Especially the BN.js reexport turned out to be very useful and somewhat needed after some experiences we made along the BN v4 to v5 switch which caused major incompatibilities when we partly switched some libraries here. The rlp reexport is not so strictly needed eventually and might be up for discussion, not really some urgency here though.

Will close.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants