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

Use more robust Array type check #513

Merged
merged 1 commit into from
Sep 21, 2018
Merged

Conversation

rubendg
Copy link
Contributor

@rubendg rubendg commented Sep 20, 2018

Improved robustness of Array type check by using isArray instead of instanceof.

I have a project with supertest (^3.1.0) and Jest (^23.0.0) as test runner. For some reason, that I haven't yet been able to pinpoint, the array instance check fails for me.

Vars in scope:

actual = ["a"]
field = "set-cookie"
fieldExpected = "a"

when I evaluate the following expressions I get:

actual instanceof Array -> false
Array.isArray(actual) -> true

changing the check to use isArray makes it work for me.

I suspect I'm running into some of the issues described over here: http://perfectionkills.com/instanceof-considered-harmful-or-how-to-write-a-robust-isarray/

Improve robustness of Array type check.
@amoshaviv
Copy link

👍

@coveralls
Copy link

coveralls commented Sep 20, 2018

Pull Request Test Coverage Report for Build 335

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.285%

Totals Coverage Status
Change from base Build 330: 0.0%
Covered Lines: 136
Relevant Lines: 140

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 335

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.285%

Totals Coverage Status
Change from base Build 330: 0.0%
Covered Lines: 136
Relevant Lines: 140

💛 - Coveralls

@rubendg rubendg changed the title Use more robust Array instance check Use more robust Array type check Sep 20, 2018
@rimiti rimiti self-assigned this Sep 20, 2018
@rimiti rimiti added this to the v3.4.0 milestone Sep 20, 2018
@rimiti
Copy link
Contributor

rimiti commented Sep 21, 2018

Thank you @rubendg for your fix.
I'll merge it in the next release. 👍

@rimiti rimiti changed the base branch from master to v3.4.0 September 21, 2018 07:07
@rimiti rimiti merged commit 8158979 into ladjs:v3.4.0 Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants