Code and interface improvements after enabling some eslint recommended rules requiring type checking

[ngbrown]

Summary of changes

• Added this: void to functions in the SessionStorage and SessionIdStorageStrategy interfaces so destructuring is correctly part of the contract.
• Added void operator to Promises that are not being awaited.
• Added internal-type-checked.js and rules/type-checked.js to remix-eslint-config to handle typescript-eslint rules that require type information and using the @typescript-eslint/parser.

Details

I attempted to add the typescript-eslint rules contained in "plugin:@typescript-eslint/recommended-requiring-type-checking" to a Remix based project and came across two common code issues:

const { pipe, abort } = renderToPipeableStream();

and then on:

const { getSession, commitSession, destroySession } = createDatabaseSessionStorage();

Both of these cause an unbound-method error:

error Avoid referencing unbound methods which may cause unintentional scoping of this.

If your function does not access this, you can annotate it with this: void, or consider using an arrow function instead @typescript-eslint/unbound-method

This error needed to be taken care of by the type definitions provided by the libraries.

@types/react-dom v18.2.4 now includes this: void in the renderToPipeableStream() function definition, so the first half is solved.

This pull request is to address the changes needed in Remix. At the same time, I tried to improve code quality by addressing some of the issues raised when enabling the type checked linting rules.

Adding this: void could subtly change the contract for external libraries returning SessionStorage, so it would be good to get this in before Remix v2.

I tried my best to incorporate the rules from plugin:@typescript-eslint/recommended-requiring-type-checking in a way that seemed consistent with the other rules, but I'm open to suggestions. Running these rules requires more memory from the node process and may take longer.

I could also split out the code changes from the linting changes and have this pull request just be the code improvements.

Testing Strategy:

I opened up my windows machine and ran this script:

yarn lint
yarn test:primary
npx playwrite install
yarn test
yarn build --tsc

[changeset-bot]

🦋 Changeset detected

Latest commit: c901bf2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages

Name	Type
@remix-run/eslint-config	Minor
@remix-run/server-runtime	Minor
remix	Minor
@remix-run/dev	Minor
@remix-run/react	Minor
@remix-run/serve	Minor
@remix-run/testing	Minor
@remix-run/cloudflare	Minor
@remix-run/deno	Minor
@remix-run/node	Minor
create-remix	Minor
@remix-run/css-bundle	Minor
@remix-run/cloudflare-pages	Minor
@remix-run/cloudflare-workers	Minor
@remix-run/architect	Minor
@remix-run/express	Minor
@remix-run/netlify	Minor
@remix-run/vercel	Minor

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

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[MichaelDeBoey]

Thanks for taking the time to add these changes @ngbrown! 🙏

However, I'm going to close this in favor of @JoshuaKGoldberg's #6248.
If he's missing some things, you can always make some comments & help him out addressing them if necessary.

[ngbrown]

Hi @MichaelDeBoey I understand my changes in remix-config-eslint would conflict with #6248, but that change doesn't turn on any type checked rules and also doesn't address the primary motivation of adding this: void to methods that are commonly destructured in the Remix documentation.

Should I open a separate pull request with just the code changes? Should I submit several pull requests with different groups of changes?

I put in three changeset files so could open two pull requests, one for this: void and the other for the void operator and await in test cleanup.

[MichaelDeBoey]

@ngbrown I'm sorry, I thought all of them were related to adding the internal ESLint ruleset

If not, could you create separate PRs for those changes please?
That would be easier to review that way.

[ngbrown]

@MichaelDeBoey I was using the eslint ruleset as the "test" to drive the changes. Sorry if that was confusing.