Skip to content
This repository has been archived by the owner on Apr 24, 2023. It is now read-only.

fix: replace node buffers with uint8arrays #244

Merged
merged 4 commits into from
Aug 12, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Aug 10, 2020

BREAKING CHANGES:

  • All deps used by this module now use Uint8Arrays in place of node Buffers

also fixes #214

BREAKING CHANGES:

- All deps used by this module now use Uint8Arrays in place of node Buffers
@achingbrain
Copy link
Member Author

Master of this repo depends on this unmerged branch: https://github.com/libp2p/js-libp2p-interfaces/tree/chore/skip-abort-while-reading-for-webrtc

I think it might need merging before the tests will pass.

@vasco-santos
Copy link
Member

We have #214 to track the dep issue. There are a couple of tests failling with the interface and we want to tackle them when we iterate on webrtc with distributed signalling.
The easiest solution now is to rebase the interface PR with its master.
I will do it

@jacobheun
Copy link
Contributor

I added a PR to aegir to let us just skip this via the cli so we can do it in travis instead of depending on that branch, ipfs/aegir#622.

I looked into the issue with the tests but will need more time to fix the root cause. The issue with the abort error not being thrown is that SimplePeer is overriding destroy on ReadableStream and doing some things that don't conform to standard Stream behavior. We're currently using a fork of SimplePeer anyway, so we should fix it there until we can get rid of ReadableStream altogether.

@vasco-santos
Copy link
Member

I added a PR to aegir to let us just skip this via the cli so we can do it in travis instead of depending on that branch, ipfs/aegir#622.

I looked into the issue with the tests but will need more time to fix the root cause. The issue with the abort error not being thrown is that SimplePeer is overriding destroy on ReadableStream and doing some things that don't conform to standard Stream behavior. We're currently using a fork of SimplePeer anyway, so we should fix it there until we can get rid of ReadableStream altogether.

I just updated the interface tag. But, I like your solution more! 👍

@jacobheun
Copy link
Contributor

The simple-peer fork needed to switch over to Uint8Arrays, so I went ahead and just fixed the underlying destroy problem. ipfs-shipyard/simple-peer#1

Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should all be fixed. The previously skipped tests are also fixed now.

@jacobheun jacobheun merged commit 68805b0 into master Aug 12, 2020
@jacobheun jacobheun deleted the fix/replace-node-buffers-with-uint8arrays branch August 12, 2020 10:54
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.

Unskip subset of libp2p-interface tests for transport
3 participants