Conversation
|
There were reasons back then for including it, fwiw. Array support just needs to be removed entirely and a new major needs to be released. |
|
Is that any time soon? If not, we could still merge this now as a patch version until that happens. |
|
Because this isn't a patch change. This is a major break. No matter how seemingly inconsequential, changing to
Whenever someone would like to file a PR - I don't have time right now. |
|
Yep that's true, though it should have never had the dependency in the first place. I'll do it. |
I understand that now that is the case, but when this module was released, it was not. There were still packages that returned Array-like objects that I wanted to support but that did not meet One of the worst offenders was (and still is) the DOM. Please don't assume malice in code. |
|
here. i read your README and this is exactly the behaviour your own docs specify (which in fact, the issue from years ago and the proposed diff did not support and actually would've broken). as the readme describes, something like this: var AdvancedError = errorEx('AdvancedError', {
foo: {
message: function (value) {
if (value) {
return 'bar ' + value;
}
return null;
}
}
})would now result in did it in the same pr because why not, its tiny either way and the same objective. |
|
@Qix- what do you wanna do about this PR? it maintains your current interfaces/functionality as far as i can see |
|
@Qix- Just following up with this. What would be the path forward for us to contribute a version that removes |
You really don't need this micro-dependency,
Array.isArraywill do just fine for your use case here.One less dependency, and now zero-dependencies overall.
im aware you were trying to remove array support a few years back but that didn't seem to go anywhere. this package is still widely used so would be nice to strip one more unnecessary bit of cruft from the dependency tree regardless.