Fix checks for image duplicates

[kellnerd]

Dealing with proper URLs instead of URL strings is not always easier, 4ae985d broke the comparison of image URLs and made the use of a Set kind of pointless:
Two URL objects for the same image URL are not considered identical, so I changed this to store and compare the URL.href attributes (of type string) instead.

In addition to that I have two other minor improvements:

• Remove the .js extension from type definition imports and adapt tsconfig.json to make the type acquisition of VS Code happy. There are still four regular imports with a "wrong" .js extension left, but removing the extension causes the build script to fail (for whatever reason).
• Simplify the calculation of the longest metadata field's length (just nitpicking, but more efficient and easier understandable IMO).

[kellnerd]

Not sure whether typeRoots makes paths redundant as you seem to use it only for type definitions and always have proper relative or absolute paths for regular module imports. I have not removed it since it probably does the trick in your IDE, but VS Code needs the typeRoots property and acts dumb without it.

[ROpdebee]

No clue. I started doing this with no TS experience, and I've never properly RTFD. The config is sub-optimal anyway, it's leading to implicitly included types which IMO should be implicitly included (like __CoverArt in https://github.com/ROpdebee/mb-userscripts/blob/main/src/lib/MB/__MB__.d.ts, which aliases another type in the src). I'll tinker with it a bit once everything slows down :)

[kellnerd]

I discovered https://www.typescriptlang.org/tsconfig/#types while I was looking for the docs of typeRoots. Probably you need to overwrite that option to disable these automatic imports.

[ROpdebee]

It's a bit weird though, since typeRoots apparently only matters for /// <reference types=".." /> directives (per here. To my knowledge, we're never using those directives.

Anyway, if it works for you, I'm fine with it :) I'll be changing the paths anyway and might be splitting up some of the tsconfig files, since I've got a feeling something's not working properly.

[kellnerd]

I am just reviewing your changes from #68 and apparently the typeRoots property is indeed not necessary (now). Removing it from tsconfig.json and restarting VS Code does no longer cause problems with relative import type statements. Something must have changed in the mean time, either your recent refactoring or me updating the dependencies has solved the problem. Anyway, I'm happy to remove it again once we are done with these huge PRs.

[ROpdebee]

Looks good to me, but could you merge in the latest main and check the wasMaximised calculation? I think I just fixed that separately when working on the edit notes.

The whole URL string -> URL conversion was mainly driven by concerns that the script would be fed URLs with parameters or hashes, e.g. https://discogs.com/release/12345?foo=bar and that such URLs might break some of the regexp checks. Using url.pathname avoids that, and it felt better to do the conversion at the earliest possible point rather than temporarily converting it all across the code (but then again, now I have to do url.href a lot, but at least I get early validation of invalid URLs, I guess)

The funny thing is that I knew full-well that URL doesn't properly do equality checks (and IMHO it's really stupid that it doesn't), I encountered that in 04c7653 :)

I've also go no clue what's with those JS extensions, probably something to do with babel, typescript, node, and the mixing of CJS and ES modules, and trying until something worked. Then changing something else, which then made all of the trial and error useless. Ugh.

[kellnerd]

Looks good to me, but could you merge in the latest main and check the wasMaximised calculation? I think I just fixed that separately when working on the edit notes.

Yes, I noticed this right before I created the pull request. But since you have changed that line in exactly the same way there are no merge conflicts with this PR. I just merged locally and it auto-merged fine indeed. I will push the merge commit now but I guess it just creates unnecessary clutter in the git history, so I can withdraw it again before you merge.

I've also go no clue what's with those JS extensions, probably something to do with babel, typescript, node, and the mixing of CJS and ES modules, and trying until something worked. Then changing something else, which then made all of the trial and error useless. Ugh.

Probably somehow related to the fact that these are (in)directly imported by the build script itself while the extensionless imports are only touched by rollup!? Anyway, never change a running system 😂

[ROpdebee]

Probably somehow related to the fact that these are (in)directly imported by the build script itself while the extensionless imports are only touched by rollup!? Anyway, never change a running system 😂

https://github.com/facebook/hhvm/blob/39415630e84538fb553cd325a85ce837bcf6cb70/src/runtime/ext/ext_imagesprite.cpp#L649

I think I need some tests as well 😛