Update dependencies and lint

[fregante]

No description provided.

[fregante]

cc @sindresorhus on these notes

[fregante]

import-js/eslint-plugin-import#1341

[sindresorhus]

That rule has other problems too. Disabled: xojs/xo@ee145cb

[fregante]

Annoying: It doesn't let us use async functions as event handlers.

[sindresorhus]

While it's convenient, my reasoning is that using async there makes it hard to see that the event handler function will cause an uncaught rejection if the await rejects. For example, using async in a new Promise can cause hard to track down issues.

And when people see:

[1, 2, 3].forEach(async value => {
  await doSomething(value);
});

they might be fooled into thinking that it awaits for each iteration, which it does not.

I'm happy to discuss this though. Maybe there some additional config that could be added to the rule?

[fregante]

I understand the value in that case, but I don't have a solution for our use-case: our handlers are async just because Promise handling is great, not because anyone expects addEventListener to await them.

If we enable this rule, we have to write listeners as:

function handler() {
	(async () => {
		document.body.append(await fetchDom(...));
	})();
}

[fregante]

A more useful related rule would be the opposite: always await async functions. Does this exist already?

async function set(){...}
async function get(){...}
set(1); // Forgot `await`
await get();

[sindresorhus]

A more useful related rule would be the opposite: always await async functions. Does this exist already?

This? https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-floating-promises.md

[fregante]

Almost, returnsPromise() is not awaited even though it returns a promise. It seems that it requires await only on variables

[fregante]

Buggy: it doesn't recognize returning a promise in arrow functions (Edit: will be fixed in the next release). It's already on top of some arguably annoying rules and it conflicts with them:

e.g.

() => Promise.resolve()
  ✖  Functions that return promises must be async. @typescript-eslint/promise-function-async

async () => Promise.resolve()
  ✖  Async arrow function has no await expression. @typescript-eslint/require-await

async () => await Promise.resolve()
  ✖  Redundant use of await on a return value. no-return-await

The first one was perfectly fine (EDIT:) and for this reason, I disabled the initial rule (2036c3e) since require-await seems to be more useful.

[fregante]

Good in theory, but makes us have to type one-off things just for the heck of it:

• Code examples:

   (window as any).select = select;

   const editor: CodeMirrorInstance = document.querySelector<any>('.CodeMirror').CodeMirror;

   (global as any).navigator = window.navigator;
   (global as any).document = window.document;
   (global as any).location = new URL('https://github.com');
  
   // Which becomes:
   (global as unknown as typeof window).navigator = window.navigator;
   (global as unknown as typeof window).document = window.document;
   (global as unknown as typeof window).location = new URL('https://github.com');

• It'd force us to type API calls

• Also kills AnyObject which is pretty much required for the API handler

[sindresorhus]

The idea was that you explicitly disable the rule where you want any to make it inconvenient enough to use that it's not misused, but I encountered some annoyance with it too, so I'll disable it for now: xojs/eslint-config-xo-typescript@bbbf889

[fregante]

The idea was that you explicitly disable the rule where you want any to make it inconvenient enough to use that it's not misused

Maybe that is worth it, I missed this one just recently: https://github.com/sindresorhus/refined-github/pull/2393/files#diff-f8fed5d366f31fce028a9c8ceca38c3eR6

The lines in my examples would just have to use eslint-disable-next-line or be wrapped by disabled/enable

[sindresorhus]

I think it will be easier to enable in a year or so when TS has matured more and we need less any in general.

[fregante]

What other problems have you had?

Do you think there will ever be a solution to my examples? They basically are:

• setting a global (without having to create a property on the Global/Window types)
• setting/getting one-off properties. It could probably already be fixed with:

   type CodeMirrorElement = Element & {CodeMirror: CodeMirrorInstance}
   const editor = document.querySelector<CodeMirrorElement>('.CodeMirror').CodeMirror;

• we just want to avoid typing of certain parts like APIs because it's extra work that has to be updated when the query changes, otherwise the API response won't actually match the types (in essence, manually typing the API is as bad as an assertion). Also the API response ALWAYS has to start from AnyObject, there's no workaround for that.

[sindresorhus]

I had problems with some bad types from Electron and React. Basically out of my control. I also had to use any for some things like this: https://github.com/sindresorhus/caprine/blob/7965b36c11ae1f7ccb2185bebdc85e9fc3d01078/source/menu.ts#L92

In the future, there will be less globals, and the types of large projects will be better, TS will be better.

[fregante]

I merged all of these TODOs into one. They're just noise since we can only wait for the underlying bugs to be fixed.

[fregante]

I'm temporarily avoiding v2 because of a bug I don't know how to work around: typescript-eslint/typescript-eslint#890

webpack.config.ts:undefined:undefined
✖ 0:0 Parsing error: If "parserOptions.project" has been set for @typescript-eslint/parser, ./webpack.config.ts must be included in at least one of the projects provided.

test/page-detect.ts:undefined:undefined
✖ 0:0 Parsing error: If "parserOptions.project" has been set for @typescript-eslint/parser, ./test/page-detect.ts must be included in at least one of the projects provided.

test/utils.ts:undefined:undefined
✖ 0:0 Parsing error: If "parserOptions.project" has been set for @typescript-eslint/parser, ./test/utils.ts must be included in at least one of the projects provided.

test/fixtures/globals.ts:undefined:undefined
✖ 0:0 Parsing error: If "parserOptions.project" has been set for @typescript-eslint/parser, ./test/fixtures/globals.ts must be included in at least one of the projects provided.

They should go away by adding test in tsconfig's includes, but that file appears to be ignored.

Also reported in xojs/eslint-config-xo-typescript#15