9

How to implement Nullable feature in ? I need to support the source code of my previous co-worker, who used too much destructuring feature of . Something like this, every where:

dispatch(
    loadImports(response.items.map(({ importRecord: { ['import']: importId } }) => importId))
)

In this example, it's possible that I may get TypeError: Cannot read property 'import' of null error.

I don't want to rewrite the whole destructures to regular conditions. Or if there isn't, how to deal with them, without rewriting?

UPD:

Expected version by co-worker: https://jsbin.com/fesewiy/edit?js,console

Current version: https://jsbin.com/qirixil/edit?js,console

xurshid29
  • 4,172
  • 1
  • 20
  • 25
  • What would the code look like if you did rewrite it - ie what should happen if you _do_ pass in null? Either the code block should handle it, in which case pure destructuring is wrong because it doesn't handle the null case, or it shouldn't handle it and should throw an error anyway, in which case you're left with changing your calling code to check for nulls before you pass them. – James Thorpe Nov 20 '17 at 10:51
  • @JamesThorpe Above example could look like this with conditions: `loadImports(response.items.map((item) => item.importRecord && item.importRecord.import && item.importRecord.import.id))` – xurshid29 Nov 20 '17 at 10:55
  • But that fundamentally changes the meaning of what the code does. Now you allow nulls to be mapped. – James Thorpe Nov 20 '17 at 10:56
  • @JamesThorpe Plus, find and fix them, too much time. – xurshid29 Nov 20 '17 at 10:56
  • @JamesThorpe It's ok, if I allow nulls, backend can handle them properly. – xurshid29 Nov 20 '17 at 10:57
  • You have to change your code anyway. As far as I know, there is no way to do it with object destructuring. But you could use an approach like this: https://stackoverflow.com/questions/14782232/how-to-avoid-cannot-read-property-of-undefined-errors/42349521#42349521 – str Nov 20 '17 at 16:16
  • Your co-worker must hate you. That is an obnoxiously unmaintainable inline function. – Patrick Roberts Jan 19 '18 at 14:41
  • Can you reproduce the issue at stacksnippets? See https://stackoverflow.com/help/mcve – guest271314 Jan 19 '18 at 15:02
  • @guest271314 Updated the post – xurshid29 Jan 20 '18 at 09:22
  • @PatrickRoberts Added samples – xurshid29 Jan 20 '18 at 09:22
  • You might check this https://stackoverflow.com/questions/48338466/destructure-object-only-if-its-truthy/48338661#48338661 – Shubham Khatri Jan 20 '18 at 09:31
  • @xurshid29 The code at https://stackoverflow.com/questions/47390089/js-destructuring-how-to-deal-with-null-or-undefined-values?noredirect=1#comment83675825_48179897 and/or https://stackoverflow.com/questions/47390089/js-destructuring-how-to-deal-with-null-or-undefined-values?noredirect=1#comment83676180_48179897 should handle the case and return expected result – guest271314 Jan 20 '18 at 15:37

2 Answers2

2

Because you are using map, you need to get some result for each item. The best solution with minimum changes will be to use only one level of destructuring and then filter the array to stay with array of values that are not falsy.

dispatch(
    loadImports(response.items
        .map(({ importRecord }) => importRecord && importRecord.import)
        .filter(v => v)
))
Edan Chetrit
  • 4,713
  • 2
  • 20
  • 20
0

You can set the value of the default parameter to a plain object, use OR operator to return either importId or null

arr.map(({ importRecord: { ['import']: importId } = {}}) => importId || null)
guest271314
  • 1
  • 15
  • 104
  • 177
  • @PatrickRoberts Note, using the approach at your comment `undefined` will be returned instead of `null`. – guest271314 Jan 19 '18 at 14:58
  • No it won't, it will throw the same error: https://jsfiddle.net/yttzcyf1/ and https://jsfiddle.net/cg1h92Lx/ – Patrick Roberts Jan 19 '18 at 14:59
  • Well, we are trying with different objects as OP did not include enough code at the question to reproduce `var arr = [{importRecord: {imported:123}} ]; arr.map(({ importRecord: { import: importId } = { import: null }}) => importId)` – guest271314 Jan 19 '18 at 15:01
  • OP included the error, which indicates that `importRecord` is sometimes `null`, not `undefined`. – Patrick Roberts Jan 19 '18 at 15:02
  • "In this example, it's possible that I may get `TypeError: Cannot read property 'import' of null` error." -- implying that the parent object of `import` which is `importRecord` is nullable. – Patrick Roberts Jan 19 '18 at 15:05
  • @PatrickRoberts Then what is the object? Cannot reproduce that exact error. – guest271314 Jan 19 '18 at 15:05
  • Again, see https://jsfiddle.net/yttzcyf1/ The "exact" error message is browser-dependent, but the cause is the same. – Patrick Roberts Jan 19 '18 at 15:06
  • That is not the _exact_ same error as described at OP. Did you cast "downvote" for an answer to a question where the code is not reproduced at OP? – guest271314 Jan 19 '18 at 15:07
  • Did you prematurely answer a question that wasn't MCVE? – Patrick Roberts Jan 19 '18 at 15:08
  • @PatrickRoberts No. Tested code around 11-20-2017. Unfortunately did not preserve the object that tested with the code at Answer. Would have to scan the drive (time consuming) to rebuild the code tried around the date that made a note of the Question to get the objects tried at that time, though will try anyway. Would not have posted unless the code returned expected result – guest271314 Jan 19 '18 at 15:10
  • Note that my implication was that if the question is not MCVE, the answer is by definition premature. It doesn't matter whether or not you tested your code. As you've stated, you're still unsure what the conditions to produce OP's error were, therefore writing an answer you're certain is correct is impossible. – Patrick Roberts Jan 19 '18 at 15:16
  • @PatrickRoberts No, was not premature as to the interpretation of the Question at the time. Again, else, would not have posted the Answer two months later. Am trying to avoid the time consuming process of filtering through the fragments in hd to contend with your assessment of what the code at OP is. Note, that your point is plausible, though you too are working from the standpoint of speculation. It may have been that interpreted the question at the time as to `import` property being null. Usually keep such tests, though did not on this occasion. – guest271314 Jan 19 '18 at 15:17
  • Again, as _you've_ just stated, you gave `an answer to a question where the code is not reproduced at OP`. That is not MCVE. Therefore, said `interpretation of the question at the time` is not 100% certain. This is the risk of interpretation. I've given you mine, and based on mine, this answer is incorrect. Accept your -2 with a little dignity and just move on. – Patrick Roberts Jan 19 '18 at 15:21
  • @PatrickRoberts It is not about "dignity". That is a feeling that is irrelevant, and no external entity could possibly effect this users' state of being whatsoever. Am concerned with the code itself. It does not matter whether the "downvote" is "accepted" or not. You already did what you did. Will try to find the code that tried which prompted the Answer. – guest271314 Jan 19 '18 at 15:23
  • @PatrickRoberts Ah, was coding for `import` not being defined `[{importRecord: {abc:123}} ].map(({ importRecord: { ['import']: importId } = {}}) => importId || null)` – guest271314 Jan 19 '18 at 15:28
  • That's a non-representative test-case since `[{importRecord: {abc:123}} ].map(({ importRecord: { ['import']: importId } }) => importId)` doesn't throw a `TypeError` as OP has indicated can occur with input. – Patrick Roberts Jan 19 '18 at 15:31
  • @PatrickRoberts Missed including the `s` in the last comment `var arr = [{importRecords: null} ]; arr.map(({ importRecord: { ['import']: importId } = {}}) => importId || null)`. Here is the code which logs same error as described at OP `arr.map(({ importRecord: { ['import']: importId } }) => importId)` given the `arr` above – guest271314 Jan 19 '18 at 15:33
  • That handles the case for `importRecord === undefined` but now we're back to the start where I said you don't handle `importRecord === null` as OP's `TypeError` indicates. – Patrick Roberts Jan 19 '18 at 15:36
  • @PatrickRoberts What are you talking about now? `importRecord` does not exist, which is also a viable interpretation of the question. Note that cannot reproduce the _exact_ error described at OP. The error is always `'undefined' or 'null'`. OP does not explicitly state that `importRecord` _is_ `null`. The same error is thrown for either case. And there is nothing that you could surmise from the original question which would distinguish between the two possibilities and settle the matter. OP needs to reproduce the code to settle the matter of which pattern returns which error – guest271314 Jan 19 '18 at 15:37
  • @PatrickRoberts Therefore, your "downvote" is unwarranted given the examples demonstrated at https://stackoverflow.com/questions/47390089/js-destructuring-how-to-deal-with-null-or-undefined-values/48179897?noredirect=1#comment83674615_48179897. You cannot conclusively draw the conclusion that this users' interpretation of the original question is incorrect. Though your interpretation of the question is duly noted and also viable. Perhaps post your own answer to the inquiry so resolve all possible cases. – guest271314 Jan 19 '18 at 15:45
  • "`importRecord` does not exist" satisfies `importRecord === undefined`. You should know this. Also, I take it you're using Chrome. In recent versions, Chrome has used a more generic message for destructuring errors, e.g. ``TypeError: Cannot destructure property `import` of 'undefined' or 'null'.``. However you can still reproduce specific cases from 2 or 3 versions ago by simply attempting `null.import` or `undefined.import`. Your argument that it cannot be reproduced _exactly_ holds no water, you're just refusing to acknowledge your answer does not solve the original problem. – Patrick Roberts Jan 19 '18 at 15:47
  • @PatrickRoberts Am not refusing to acknowledge anything. We simply interpreted the question differently. Sustain the "downvote" if that satisfies to match your critique of the answer. Though you cannot refute that the answer does in fact handle the case of `importRecord` not existing in the first instance, as demonstrated at https://stackoverflow.com/questions/47390089/js-destructuring-how-to-deal-with-null-or-undefined-values/48179897?noredirect=1#comment83674615_48179897. Your perspective lends another case to the inquiry which did not consider at the time. You should post your own answer. – guest271314 Jan 19 '18 at 15:51
  • OP's error `TypeError: Cannot read property 'import' of null` cannot possibly occur in _any_ implementation of JavaScript when the condition "`importRecord` does not exist" occurs, otherwise the error would be `TypeError: Cannot read property 'import' of undefined` or ``TypeError: Cannot destructure property `import` of 'undefined' or 'null'.`` depending on the implementation. Your interpretation is incorrect. Period. – Patrick Roberts Jan 19 '18 at 15:54
  • @PatrickRoberts Ok, let us take a step back. What are you suggesting is the object which creates the error? – guest271314 Jan 19 '18 at 15:59
  • @PatrickRoberts This should handle that case `[{ importRecord: null }].map(({importRecord} = {}) => { let {import:importId} = importRecord || {}; return importId}) // [undefined]` – guest271314 Jan 19 '18 at 16:07
  • @PatrickRoberts Any objections or considerations not seeing here as to the above pattern? – guest271314 Jan 19 '18 at 16:12
  • No objections, though it does return `undefined` instead of `null`. I thought you wanted it to return `null`. Just to clarify, that's not an objection, just an observation. Also an equivalent to your above suggestion would be `[{ importRecord: null }].map(({importRecord} = {}) => (importRecord || {}).import)` – Patrick Roberts Jan 19 '18 at 16:16
  • @PatrickRoberts Independent observations as to code posted at answer are certainly welcome here. Where does OP specify what should be returned for that case? – guest271314 Jan 19 '18 at 16:20
  • OP does not specify, that's why I explicitly stated it wasn't an objection. I just observed that your current answer prefers to output `null` [like you said](https://stackoverflow.com/questions/47390089/js-destructuring-how-to-deal-with-null-or-undefined-values/48179897?noredirect=1#comment83673194_48179897). – Patrick Roberts Jan 19 '18 at 16:21
  • @PatrickRoberts Now OP has solutions for at least two cases. That is, if the actual code reflects what is currently described at the original question. – guest271314 Jan 19 '18 at 16:23