Skip to content

fix(isDeepKey): improve deep key detection to match strict patterns #1621

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.

Sign up for GitHub

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

Open
ramong26 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(isDeepKey): improve deep key detection to match strict patterns #1621

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04274f0
fix(isDeepKey): improve deep key detection to match strict patterns
ramong26 Feb 17, 2026
da895cc
fix(findIndex): Revert changes: remove files that were incorrectly mo…
ramong26 Feb 17, 2026
6bec8ec
fix(isKey): align regexIsDeepProp usage with internal deep key detection
ramong26 Feb 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/compat/_internal/isDeepKey.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,23 @@ describe('isDeepKey function', () => {
it('returns false for non-deep keys', () => {
expect(isDeepKey('a')).toBe(false);
expect(isDeepKey(123)).toBe(false);
expect(isDeepKey(Symbol('a'))).toBe(false);
});

it('returns false for invalid deep key patterns', () => {
expect(isDeepKey('a.')).toBe(false);
expect(isDeepKey('.a')).toBe(false);
expect(isDeepKey('a[b')).toBe(false);
expect(isDeepKey('a]b]')).toBe(false);
expect(isDeepKey('a][b')).toBe(false);
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated tests cover a handful of invalid patterns, but they don't cover several false positives the new implementation still allows (e.g. a..b, a.b., a[b][c, a[b]], a[b]c, a.b[). Adding cases like these would prevent regressions and ensure the “strict pattern” behavior described in the PR.

Suggested change
expect(isDeepKey('a][b')).toBe(false);
expect(isDeepKey('a][b')).toBe(false);
expect(isDeepKey('a..b')).toBe(false);
expect(isDeepKey('a.b.')).toBe(false);
expect(isDeepKey('a[b][c')).toBe(false);
expect(isDeepKey('a[b]]')).toBe(false);
expect(isDeepKey('a[b]c')).toBe(false);
expect(isDeepKey('a.b[')).toBe(false);

Copilot uses AI. Check for mistakes.
});

it('returns false for empty string', () => {
expect(isDeepKey('')).toBe(false);
});

it('returns true for valid bracket patterns', () => {
expect(isDeepKey('a[0]')).toBe(true);
expect(isDeepKey('[a]')).toBe(true);
});
});
27 changes: 21 additions & 6 deletions src/compat/_internal/isDeepKey.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,30 @@
* isDeepKey(123) // false
* isDeepKey('a.b.c') // true
* isDeepKey('a[b][c]') // true
* isDeepKey('a.') // false
* isDeepKey('.a') // false
* isDeepKey('a[b') // false
* isDeepKey('a]b]') // false
* isDeepKey('a][b') // false
*/
export function isDeepKey(key: PropertyKey): boolean {
switch (typeof key) {
case 'number':
case 'symbol': {
return false;
if (typeof key === 'number' || typeof key === 'symbol') {
return false;
}

if (typeof key === 'string') {
const dotIndex = key.indexOf('.');
if (dotIndex > 0 && dotIndex < key.length - 1) {
return true;
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dot check only looks at the first '.' and only enforces it isn't the first/last character. This still returns true for syntactically invalid dot paths like a..b (empty segment) or a.b. (trailing dot), which contradicts the PR goal of accepting only strict deep-key patterns. Consider validating the full string (e.g., reject consecutive dots / any empty segment) rather than just indexOf('.') bounds checks.

Suggested change
if (dotIndex > 0 && dotIndex < key.length - 1) {
return true;
if (dotIndex !== -1) {
const segments = key.split('.');
if (segments.length > 1 && segments.every(segment => segment.length > 0)) {
return true;
}

Copilot uses AI. Check for mistakes.
}
case 'string': {
return key.includes('.') || key.includes('[') || key.includes(']');

let leftBracketIndex = key.indexOf('[');
let rightBracketIndex = key.indexOf(']');
if (leftBracketIndex !== -1 && rightBracketIndex !== -1 && leftBracketIndex < rightBracketIndex) {
return true;
}
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bracket check only verifies that the first '[' appears before the first ']'. It will still return true for invalid/mismatched bracket patterns like a[b][c (unclosed), a[b]] (extra ']'), a.b[ (unclosed), or a[b]c (missing separator). To meet the strict-pattern intent, this likely needs a small parser/regex that validates balanced bracket segments and disallows stray brackets outside of them.

Copilot uses AI. Check for mistakes.

return false;
}
return false;
}