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.

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

_.take(n) (and others) do not type check their arguments #594

Open
k0pernikus opened this issue Feb 8, 2017 · 1 comment
Open

_.take(n) (and others) do not type check their arguments #594

k0pernikus opened this issue Feb 8, 2017 · 1 comment
Labels

Comments

@k0pernikus
Copy link

k0pernikus commented Feb 8, 2017

Using highland@2, given this example:

"use strict";
const Highland = require("highland");
let stream = Highland([1, 2, 3, 5, 6, 7, 8, 9, 10]);
let take3 = stream.take(3);
take3.toArray((threeItems) => {
    console.log('will be three', threeItems);
});
stream = Highland([1, 2, 3, 5, 6, 7, 8, 9, 10]);
take3 = stream.fork().take('3');
take3.toArray((threeItems) => {
    console.log('will be all', threeItems);
});

the output will be:

will be three [ 1, 2, 3 ]
will be all [ 1, 2, 3, 5, 6, 7, 8, 9, 10 ]

As a developer using highland, I expect highland to throw an error if a false input parameter is provided. I might also expect it to infer the proper value from the faulty input (and at best issue a warning nontheless.)

I absolutely do not expect highland to just "ignore" the command and return all the items from the stream.

This may apply also to other function and the bug report may be generalized as:

Each highland function should validate its input throw errors if a false type is given, it may graciously try to infer the proper type, yet if that fails, and error should be thrown.

(I stumbled across this error from passing in an integer as a command line argument ./node my-script --limit 10 via commander, and I assumed the entire time I was injecting an integer. I even typehinted for an integer within my typescript codebase, yet as the typecheck only happens on compile time, I had a hard time figuring out the source of this issue until I started a remote debugger session.)

@vqvu vqvu added the 3.x label Feb 8, 2017
@vqvu
Copy link
Collaborator

vqvu commented Feb 8, 2017

I agree that this behavior is needlessly confusing, and if I had to do it again, I would have asked for fail-fast errors. I also agree on consistent type checking across all operators. Currently, some of them do, and some of them don't, which is a problem.

Unfortunately, for take specifically, this behavior is currently documented, so I don't think I can change it without forcing a major version bump, which I don't want to do right now, since we have a more substantial 3.0 in progress. I am happy to add this behavior to the current master, but you won't see it on the 2.x releases.

@vqvu vqvu changed the title _.take(n) will take all items if n is passed as string _.take(n) (and others) do not type check their arguments Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants