Enable noUncheckedIndexedAccess
#1175
Conversation
🦋 Changeset detectedLatest commit: 4cf0b2b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -0,0 +1,35 @@ | |||
| --- | |||
| 'skuba': major | |||
There was a problem hiding this comment.
minor or major? 🤔 I think because it will likely be a breaking change for alot of repos we should probably make it major
There was a problem hiding this comment.
Oof, I'm kinda hot and cold on this change now that the tsconfig-seek upgrade has propagated to a couple of our repos and I'm seeing this PR as well 😅. It seems like there are many common scenarios that are not inferred. For example:
const result = /^([a-z])/.match(input);
if (result) {
// !!!
result[1];
}
// !!!
''.split('#')[0];
const xs: Array<{key: string, value: string}>;
if (xs.length === 1) {
// !!!
const x = xs[0];
const record = Object.fromEntries(xs.map(x) => [x.key, x.value]);
// !!!
record[x.key];
}I don't blame TypeScript here but I worry about the DX on generally well-behaved codebases once we enforce this option. We need to make sure that we can somehow deliver this without normalising usage of easy escape hatches like !; what we don't want is for this change to end up proliferating unsafe practices, and beyond just indexed access.
| if (!modulePath) { | ||
| return `${chalk.bold(value)} is an invalid module path`; | ||
| } |
There was a problem hiding this comment.
Is this even possible? The only way I've been able to arrive at an empty array is by doing ''.split('').
src/api/git/reset.int.test.ts
Outdated
| @@ -44,7 +44,7 @@ describe('soft', () => { | |||
|
|
|||
| const directory = await fs.promises.readdir(dir); | |||
|
|
|||
| expect(commits[0].oid).toEqual(initialCommit); | |||
| expect(commits[0]?.oid).toEqual(initialCommit); | |||
There was a problem hiding this comment.
It's unfortunate that this applies to tests too; ideally we'd have an ESLint rule so we could disable it on test files.
It's a bit of a wash in this example, but I'd be open to use of ! in tests. A runtime error is harmless here and would equally indicate that the test failed.
Co-authored-by: Ryan Ling <[email protected]>
…nto tsconfig-indexed-access
Release seek-tsconfig 2.0