Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

fix: add buffer #50

Merged
merged 1 commit into from
Apr 1, 2020
Merged

fix: add buffer #50

merged 1 commit into from
Apr 1, 2020

Conversation

hugomrdias
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Apr 1, 2020

Codecov Report

Merging #50 into master will increase coverage by 1.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
+ Coverage   84.61%   85.71%   +1.09%     
==========================================
  Files           1        1              
  Lines          13       14       +1     
==========================================
+ Hits           11       12       +1     
  Misses          2        2              
Impacted Files Coverage Δ
src/index.js 85.71% <100.00%> (+1.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 856bd53...658e2d4. Read the comment docs.

@vmx vmx merged commit b4e748c into master Apr 1, 2020
@vmx vmx deleted the fix/add-buffer branch April 1, 2020 11:41
@mikeal
Copy link
Contributor

mikeal commented Apr 1, 2020

What does this fix?

Isn’t the buffer module already pulled in to polyfill the Buffer API by bundlers? This potentially introduces a condition in which there is more than one Buffer module in a bundle due to version discrepancies.

@hugomrdias
Copy link
Member Author

Isn’t the buffer module already pulled in to polyfill the Buffer API by bundlers?

this will be removed by bundlers plus current polyfill are outdated

This potentially introduces a condition in which there is more than one Buffer module in a bundle due to version discrepancies.

this was already happening cause same module explicitly require others don't

@mikeal
Copy link
Contributor

mikeal commented Apr 1, 2020

sigh...

I’d love to get off of Buffer but it’s proving to be much more difficult than I had anticipated. The vast majority of the polyfill is never used but actually removing an interface that is passed around so much has proven impractical.

@hugomrdias
Copy link
Member Author

me too i have being looking at your bytesish module a lot, but as you said its hard plus we are now using bufferlist in multiple places.
would love to brainstorm this

@mikeal
Copy link
Contributor

mikeal commented Apr 2, 2020

ya, i’ve used bytesish all over the place but it doesn’t quite solve the problem, it just moves around the responsibility and makes it a little easier to not rely on anything.

maybe we should start a thread somewhere about all the different approaches we might consider. i’m sure @rvagg has thought about this quite a bit as well, and there are a few things that bl does that are missing from all the binary interfaces we have available.

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

Successfully merging this pull request may close these issues.

3 participants