Skip to content

ref(lint): no-non-null-assertion in scraps#115165

Merged
TkDodo merged 6 commits into
masterfrom
tkdodo/ref/scraps-no-non-null-assertion
May 12, 2026
Merged

ref(lint): no-non-null-assertion in scraps#115165
TkDodo merged 6 commits into
masterfrom
tkdodo/ref/scraps-no-non-null-assertion

Conversation

@TkDodo
Copy link
Copy Markdown
Collaborator

@TkDodo TkDodo commented May 8, 2026

enable the no-non-null-assertion eslint rule for scraps.

Note: I’ve ignored test files because 🤷

@TkDodo
Copy link
Copy Markdown
Collaborator Author

TkDodo commented May 8, 2026

bugbot run

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 8, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.49%

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9a1540a. Configure here.

Comment thread static/app/components/core/hotkey/useHotkeys.spec.tsx Outdated
@TkDodo TkDodo marked this pull request as ready for review May 8, 2026 11:45
@TkDodo TkDodo requested review from a team as code owners May 8, 2026 11:45
Comment on lines +241 to +244
for (let j = startingIndex; j < item.options.length; j++) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
hiddenOptionsSet.add(item.options[j]!.key);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
for (let j = startingIndex; j < item.options.length; j++) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
hiddenOptionsSet.add(item.options[j]!.key);
}
for (const option of item.options.slice(startingIndex)) {
hiddenOptionsSet.add(option.key);
}

Comment on lines 177 to 178
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
let initials = Array.from(words[0]!)[0]!;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
let initials = Array.from(words[0]!)[0]!;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
let initials = Array.from(words[0] ?? '')[0] ?? '';

Comment on lines 180 to 181
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
initials += Array.from(words[words.length - 1]!)[0]!;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
initials += Array.from(words[words.length - 1]!)[0]!;
initials += Array.from(words[words.length - 1] ?? '')[0] ?? '';

Comment on lines +262 to 263
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return value!;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hm can't we do something like?

if (value === undefined) {
  Sentry.captureMessage('useResponsivePropValue: no defined breakpoint resolved', {
      extra: {prop},
    });
    return undefined as ResponsiveValue<T>;
  }

  return value;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think this is fine as-is, but I addressed your other findings 🙏

@priscilawebdev
Copy link
Copy Markdown
Member

Can't wait for this to land 🎉 I'm not a fan of ! either.

Copy link
Copy Markdown
Member

@priscilawebdev priscilawebdev left a comment

Choose a reason for hiding this comment

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

🚀

@TkDodo TkDodo enabled auto-merge (squash) May 12, 2026 08:56
@TkDodo TkDodo merged commit 8279199 into master May 12, 2026
72 checks passed
@TkDodo TkDodo deleted the tkdodo/ref/scraps-no-non-null-assertion branch May 12, 2026 09:02
nikkikapadia pushed a commit that referenced this pull request May 12, 2026
enable the `no-non-null-assertion` eslint rule for `scraps`.

Note: I’ve ignored test files because 🤷
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants