solve 0 key see as no key #142
Conversation
Actualy when a key is set to 0, the firebase-document think that there is no key, but firebase can self generate key at 0 from array, and the concerned item will be untouchable.
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
CLAs look good, thanks! |
firebase-document.html
Outdated
| } | ||
|
|
||
| if (key) { | ||
| if (key !== undefined) { |
There was a problem hiding this comment.
Please use typeof key !== 'undefined' instead.
There was a problem hiding this comment.
It’s locally defined as a function parameter so it’s perfectly fine to use direct comparison here.
|
Can you also add a breaking test case that is fixed by your PR? |
|
Thanks for the answer, I did the check change, for the test i'm still new to polymer and firebase, and not exactly sure on how to do it for now. I'll look it up tonight to learn and will do it tomorrow if i think i'm good to go or ask for help if not :) |
|
Ok, I looked into the test things, and I'm not sure of what I have to do. It seems that the firebase-document tests are from an external lib. |
|
@arfost You add firebase-document test file in the tests directory |
firebase-document.html
Outdated
| } | ||
|
|
||
| if (key) { | ||
| if (key !== undefined) { |
|
I think we can add a test later? I'm going to check up on this, this weekend |
|
I’ll add a test for this in the coming days so we can proceed. I’d also like to revert the change requested by @mbleigh per #142 (comment) Is it ok? |
6 participants
Actualy when a key is set to 0, the firebase-document think that there is no key, but firebase can self generate key at 0 from array, and the concerned item will be untouchable.