Remove isarray dep.

[jdalton]

I know it's been brought up a few times (#212) but I think it's time again.

The isarray package is being deprecated since IE < 11 is no longer supported by MS & IE 8 was the last browser that needed it.

The Array.isArray method is supported by all Node versions, Rhino, iOS, Android, and headless browser enviros (like PhantomJS v1, SlimerJS).

[calvinmetcalf]

yeah it's probably time, do you mind reverting the changes to lib/_stream_readable.js ? the next time we build the changes will get incorporated in

[jdalton]

do you mind reverting the changes to lib/_stream_readable.js ?

Happy to make any tweaks.
Just to clarify you want all changes to lib/_stream_readable.js removed? Won't that break things?
I based the PR on https://github.com/nodejs/readable-stream/pull/214/files

[calvinmetcalf]

eh good point, so @nodejs/streams do we call this a major version as it's strictly speaking not backwards compat? or do we call it an internal api change that doesn't need a major version bump

[mcollina]

IMHO we should have a conversation about versioning in readable-stream.

This is semver-major for me.

[calvinmetcalf]

ah yes, now I remember why I didn't want to remove it before

On Fri, Sep 23, 2016 at 2:06 PM Matteo Collina notifications@github.com
wrote:

IMHO we should have a conversation about versioning in readable-stream.

This is semver-major for me.

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#234 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABE4nxXYaFhpOYiBOaK-xlz1hOFcp8WFks5qtBU3gaJpZM4KFKQs
.

[kgryte]

Instead of breaking backwards compatibility, why not just inline the module source, rather than use an external dependency?

This should quiet the calls for removing what some believe to be an "unnecessary" dependency, while at the same time not require a major version change.

I, for one, see removing support for old browsers, in this particular case, as being unnecessary.

[calvinmetcalf]

we actually probably don't work in IE8 at the moment anyway so this might not technically be a semver major, because we already did the semver major thing and nobody noticed

[mcollina]

That's why we should have a conversation about versioning. At least it is clear to us and to the community.

[kgryte]

@calvinmetcalf A distinction exists between intent and actuality. If using isarray was intended to address backward compatibility concerns, the spirit of the package would be to work in older environments, regardless of whether this is the case in practice. Hence, removing the dependency (without inlining) should be a semver major, as the intent of the package would be to not support older environments (the intent would be to break compatibility). That, by defn, would be a semver major.

But, as I stated before, I think the intent to drop backward compatibility is unwarranted.

[mcollina]

Even if I agree with you, I still think we should talk about the versioning stragegy of this one, currently there is none.

[jdalton]

@mcollina
Awesome! Moving this to a major bump is 👌
Ironing out versioning is a win!

[calvinmetcalf]

we could inline it as a short term solution to avoid the dep which people seem to really not like, while we discus versioning

[calvinmetcalf]

at the wg meeting (#235) we decided on inlining (#238) as having all the benefit with minimal (even theoretical) downside