ethjs-abi icon indicating copy to clipboard operation
ethjs-abi copied to clipboard

Why did you use exact versions of common dependencies such us BN.js

Open serjout opened this issue 8 years ago • 6 comments

In package.json for all dependencies defined exact version. It is actuall for all ethjs-* libs and produce unoptimized code in client bundle.

What is reason for this?

serjout avatar Nov 09 '17 09:11 serjout

BN should be the same across the libs, if not we can fix that for sure.

As for the other packages, I and others prefer version hardening to avoid unwanted or untested changes across packagers. If a lower level packages gets updated it should be slowly brought up and tested through each module carefully.

It's slower, but more precise and that is the goal in the end.

Sent from my iPhone

On Nov 9, 2017, at 4:43 AM, Sergey Utkin [email protected] wrote:

In package.json for all dependencies defined exact version. It is actuall for all ethjs-* libs and produce unoptimized code in client bundle.

What is reason for this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero avatar Nov 09 '17 15:11 SilentCicero

Every stable and production ready lib should follows semver convention. If you wary about unexpected bugs, you can write tests for critical code parts.

In current situation my client bundle contains >3 different copy of BN.js. And this copies are compatible via semver convention. It is not good, cause increase bundle size and time to parse and execute code.

But thank you for your answer.

serjout avatar Nov 10 '17 13:11 serjout

So BN.js, can float in actually more concerned with he ethjs libraries themselves. We can fix the BN.js issue, can you trace which modules have the different versions, we can go from there.

Sent from my iPhone

On Nov 10, 2017, at 8:15 AM, Sergey Utkin [email protected] wrote:

Every stable and production ready lib should follows semver convention. If you wary about unexpected bugs, you can write tests for critical code parts.

In current situation my client bundle contains >3 different copy of BN.js. And this copies are compatible via semver convention. It is not good, cause increase bundle size and time to parse and execute code.

But thank you for your answer.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero avatar Nov 10 '17 14:11 SilentCicero

@serjout any idea on what modules we can clean that up pretty easily.

SilentCicero avatar Nov 13 '17 22:11 SilentCicero

Now I tell you cool story, folks. There is one different reason. [email protected] has misspelling at line 53 - "require('buf' + 'fer')" instead of "require('buffer')". It is not a quite big problem, but not in my case. ethjs-abi is in dependencies tree of my project. But it runs on react-native, which use babel for resolving dependencies. And babel can not resolve dynamic require strings. So, I received error. And I need greater version of bn.js

oleg-deezus avatar Jan 11 '18 11:01 oleg-deezus

This needs to be addressed. Your absolutely right. I do not like dynamic dependencies. I can offer a bounty to fix this.

On Thu, Jan 11, 2018 at 6:06 AM, Oleg Dizus [email protected] wrote:

Now I tell you cool story, folks. There is one different reason. [email protected] has misspelling at line 53 - "require('buf' + 'fer')" instead of "require('buffer')". It is not a quite big problem, but not in my case. ethjs-abi is in dependencies tree of my project. But it runs on react-native, which use babel for resolving dependencies. And babel can not resolve dynamic require strings. So, I received error. And I need greater version of bn.js

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ethjs/ethjs-abi/issues/14#issuecomment-356902343, or mute the thread https://github.com/notifications/unsubscribe-auth/AJWhXl7Fpz-E96NooK7SLvey59Vp1776ks5tJesxgaJpZM4QXrrw .

SilentCicero avatar Jan 11 '18 18:01 SilentCicero