-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
🐛 Prevents erroring on null vals #2799
Conversation
Hey, good catch! Lines 4004 to 4015 in eb32358
|
Implemented that. I couldn't see easily how to do a test as sufficiently high level for how the tests are set up of that. |
@ekwoka the |
Yes, I will try to identify it. I just couldn't really tell where a proper test of that feature would be or what it would look like. Thanks for the feedback, I will try to identify that spot, validate the error, and add the test. |
Okay! Got it! I removed the fix on the formdata proxy, validated that this test fails (from the throw), and that the fix works. |
8c03646
to
d936b05
Compare
d936b05
to
1d776c2
Compare
Any updates or concerns? |
nope just me moving slow, thank you! |
Description
This prevents accessing the
forEach
key onnull
orundefined
values.null
is valid in JSON, so it should be supported by thehx-vals
attribute. This is a regression from 1.xCould use optional chaining, but I'd imagine that isn't an accepted feature. I also assume the fact it doesn't use
Array.isArray
is because it wants to support other containers that implementforEach
. So just doing a truthy test is satisfactory, since there is 0% chance that someone is going to be passingdocument.all
as a value.Corresponding issue:
Testing
Added test. Change is common sense, but the test ensures it's condition into the future.
Checklist
master
for website changes,dev
forsource changes)
approved via an issue
npm run test
) and verified that it succeeded