Fix TypeError error "Cannot destructure property..." in custom-property-pattern

[mattxwang]

Which issue, if any, is this issue related to?

Closes #5972.

Is there anything in the PR that needs further explanation?

In the issue description, @jeddy3 mentions to check if nodes exists before restructuring. My understanding is that nodes will always exist, and the bug arises from if the array is empty (and thus, nodes[0] doesn't exist). So, this approach checks if nodes is an empty array, and returns early if so. So, this approach checks if nodes[0] exists instead.

I've also added a test case (that previously failed with the same error, and now is resolved).

[ybiquitous]

@mattxwang Thanks for creating the pull request. 👍🏼

I think we can use the noUncheckedIndexedAccess option of TypeScript to detect such a potential bug:

--- a/tsconfig.json
+++ b/tsconfig.json
@@ -9,6 +9,7 @@
 		"strict": true,
 		"noFallthroughCasesInSwitch": true,
 		"noUnusedParameters": true,
+		"noUncheckedIndexedAccess": true,
 		"esModuleInterop": true,
 		"resolveJsonModule": true,
 		"typeRoots": ["./types", "./node_modules/@types"]

Then, run npm run lint:types:

$ npm run lint:types | grep custom-property-pattern
lib/rules/custom-property-pattern/index.js(61,13): error TS2339: Property 'value' does not exist on type 'Node | undefined'.

If we would turn on the option, the following might be better:

const firstNode = nodes[0];

if (firstNode && check(firstNode.value)) return;

Any thoughts?

[mattxwang]

Thanks for the suggestion!

If we would turn on the option, the following might be better:

const firstNode = nodes[0];

if (firstNode && check(firstNode.value)) return;

Any thoughts?

I like how concise this is. Just to confirm, should the early return look more like

const firstNode = nodes[0];

if (!firstNode || check(firstNode.value)) return;

In your suggestion, the original error fails, since firstNode is undefined (and so the checker doesn't early-exit). Or, did you mean something else?

I think we can use the noUncheckedIndexedAccess option of TypeScript to detect such a potential bug:

I think this is a good idea. I know @jeddy3 mentioned in the original issue

Follow-up PR to add "empty functions" to commonly overlooked edge cases.

Perhaps I can create a follow-up PR to enable the TS rule, resolve the other issues (umbrella-issue style), and add it to the edge case list?

[ybiquitous]

In your suggestion, the original error fails, since firstNode is undefined (and so the checker doesn't early-exit). Or, did you mean something else?

Oh, my suggestion code is wrong. 😅 !firstNode || check(firstNode.value) is right. 👍🏼

Perhaps I can create a follow-up PR to enable the TS rule, resolve the other issues (umbrella-issue style), and add it to the edge case list?

Agree. The current number of errors:

$ npm run lint:types
...
Found 148 errors in 55 files.
...

[ybiquitous]

I've created the draft pull request #5983 to turn on noUncheckedIndexedAccess.

[mattxwang]

Great, sounds good - have pushed a copy of your suggested change, thanks for the help! If we think this approach works, I can work on eventually getting #5983 to have no errors 😊

[ybiquitous]

LGTM! 👍🏼

[jeddy3]

LGTM too, thanks!

[jeddy3]

• Fixed: custom-property-pattern TypeError for "Cannot destructure property..." (#5982).