TypeScript Cleanup - Bye, bye TODOs

[nickytonline]

@bfred-it, just some more post #1735 TODO cleanup. I removed the tiny-version-compare types. The package needs to be bumped in the project, but just waiting for fregante/tiny-version-compare#7 to be merged before going ahead.

[fregante]

Instead of many tiny PRs let’s reuse this one for any TS-related changes this week

[fregante]

npm i intervalometer@latest tiny-version-compare@latest shorten-repo-url@latest

[fregante]

This TODO should be updated to point to #1936 (comment)

[nickytonline]

This should be good to go. TODOs that remain have had comments updated.

[fregante]

Any other TODOs across the extension? There are a few more type-related comments I think

[nickytonline]

Ahh, yeah. True that. I had only globals.d.ts on the brain... 🙃

[fregante]

I'm surprised to see that the todos are actually already gone. The last one would be this:

// TODO: type target asap and drop `as Element`
const observer = new MutationObserver(([{target}]) => {
	if (!select.exists('.js-details-dialog-spinner', target as Element)) {
		improveShortcutHelp(target as Element);
		fixKeys(target as Element);
		observer.disconnect();
	}
});

Is it possible without adding const dialog: Element = target?

[nickytonline]

I was working on the last todo last night. No success yet but basically the MutationCallback interface orMutationRecord interface needs to be altered via a merge declaration. Altering a MutationRecord to make target an Element doesn't seem to work and I was having issues changing the MutationCallback.

I'll take another stab at it tonight and report back.

[fregante]

Are you trying to change the type of target globally? I don’t think that’s right because target can also be a regular text node.

I think a shortcut would be to add && target instanceof Element to the existing if

[nickytonline]

Alright, a little lunchtime break/commit and I think this is good to go @bfred-it.

[fregante]

Excellent!

[fregante]

I guess the last step would be to also convert the webpack.config.js (which I was initially against) but it's not necessary. I just pushed abca820 to avoid some TS/eslint conflicts in there

[nickytonline]

Happy to convert it if you want. I've done it a bunch of times before.

[fregante]

✌️