Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add pull-debounce #27

Merged
merged 1 commit into from
Mar 11, 2017
Merged

Add pull-debounce #27

merged 1 commit into from
Mar 11, 2017

Conversation

amsross
Copy link
Contributor

@amsross amsross commented Mar 11, 2017

Having not seen this in the docs, I ended up creating this same functionality (http://github.com/amsross/pull-debounce). Upon attempting to publish it to npm, I found that @regular had already done this at http://github.com/regular/pull-debounce.

Might be helpful to someone else to be here.

@dominictarr
Copy link
Member

thanks! yes good idea!

@dominictarr dominictarr merged commit 820a6f5 into pull-stream:master Mar 11, 2017
@regular
Copy link
Contributor

regular commented Mar 12, 2017

pull-debounce has some kind of issue though, I remember vaguely. It eats up the first event it was supposed to let through or something. That"s why I never added it here myself. Is your implementation identical to mine? Would be worth comparing.

@amsross
Copy link
Contributor Author

amsross commented Mar 12, 2017

@regular my approach isn"t identical, but it swallows any late data. I"ll take a look at both for completeness later on.

@regular
Copy link
Contributor

regular commented Mar 12, 2017 •
edited
Loading

@amsross I actually prefer your implementation and it passes my tests. Do you want to make a pull request for your version of index.js? Alternatively I could npm unpublish, I guess, whichever you prefer.

@regular
Copy link
Contributor

regular commented Mar 12, 2017

@amsross oh, sorry, I missed an only, here"s the output of my tests with your index.js

TAP version 13
# should ignore frquent updates
0
1
ok 1 (unnamed assert)
not ok 2 should be equivalent
  ---
    operator: deepEqual
    expected: [ 0, 2, 3, 5 ]
    actual:   [ 0, 1 ]
  ...
[ 0, 1 ]
ok 3 (unnamed assert)
not ok 4 should be equivalent
  ---
    operator: deepEqual
    expected: [ 0, 2, 3, 5 ]
    actual:   [ 0, 1 ]
  ...
[ 0, 1 ]
not ok 5 .end() called twice
  ---
    operator: fail
  ...
# should pass through single item
ok 6 (unnamed assert)
not ok 7 should be equivalent
  ---
    operator: deepEqual
    expected: [ 0 ]
    actual:   []
  ...
[]
# should pass through single item after timeout
0
ok 8 should be equal
# should pass through late last item
1
ok 9 (unnamed assert)
not ok 10 should be equivalent
  ---
    operator: deepEqual
    expected: [ 1, 2 ]
    actual:   [ 1 ]
  ...
[ 1 ]
ok 11 (unnamed assert)
not ok 12 should be equivalent
  ---
    operator: deepEqual
    expected: [ 0, 2, 3, 5 ]
    actual:   [ 0, 1 ]
  ...
[ 0, 1 ]
not ok 13 .end() called twice
  ---
    operator: fail
  ...
ok 14 (unnamed assert)
not ok 15 should be equivalent
  ---
    operator: deepEqual
    expected: [ 1, 2 ]
    actual:   [ 1 ]
  ...
[ 1 ]
not ok 16 .end() called twice
  ---
    operator: fail
  ...

1..16
# tests 16
# pass  7
# fail  9

@regular
Copy link
Contributor

regular commented Mar 12, 2017

With my own index.js, a single test fails, that"s the issue I was talking about:

# should pass through single item after timeout
not ok 5 should be equal
  ---
    operator: equal
    expected: 1
    actual:   0
    at: Timeout._onTimeout (/Users/regular/dev/pull/pull-debounce/test.js:68:7)
  ...
``

@amsross amsross mentioned this pull request Mar 13, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amsross @dominictarr @regular