ROpdebee / mb-userscripts

Collection of userscripts for MusicBrainz
MIT License
117 stars 9 forks source link

Better modularisation and distribution #20

Open ROpdebee opened 3 years ago

ROpdebee commented 3 years ago

I hate how some of these scripts are so large, they really need to be modularised better. Maybe take inspiration from @kellnerd's repo?

If I had better modularisation, adding more providers to the work code importer would be a lot easier.

Roadmap

Scripts to migrate

ROpdebee commented 3 years ago

Sorry for the mention kellnerd, take my jealousy as a compliment. 🙂

kellnerd commented 3 years ago

No problem, since I am "watching" your repo I would have received a notification in any case. Thanks for the compliment, which I can gladly return. It's amazing how quickly you release new user scripts and to see you use some JS APIs I've never heard of before 😄

Ironically, only the userscripts themselves are modular, whereas the build script is currently one big file with many default values fitted to my current requirements. I already have it on my todo list to also modularize and generalize the build framework, but so far there was no urgent reason to approach the task. That may change since I am slowly developing a new userscript which is not going to have a bookmarklet version and hence will require changes to the build process. In case you are interested I could even split these build tools into their own repo to allow others to reuse them.

ROpdebee commented 3 years ago

JS APIs I've never heard of before

That's another reason why modularisation would be interesting, it would make it a lot easier to possibly transpile the code.

a new userscript

Do tell :)

kellnerd commented 3 years ago

it would make it a lot easier to possibly transpile the code.

I have also briefly thought about this regarding https://github.com/kellnerd/musicbrainz-bookmarklets/issues/11 but decided not to do this yet as the benefit is only minimal so far.

Do tell :)

Now we are getting off-topic, but since you were asking 😏

It is MusicBrainz: Batch‐edit release groups, a feature I wanted to have for a long time to fix inconsistent RG types for audiobooks etc. Currently it allows you to edit all selected RGs on an artist's overview page at once, but now that I have a working concept, it should not be too hard to adapt the code to work also for other entity types.

The current UI is very power user oriented and allows you to enter raw edit data (I am using a JSON representation to make it simpler/structured) which is used to generate the POST data for MBS. I had not published this one so far because it makes it very easy to submit unwanted changes and possibly lose values if you are not making the edits votable. In order to allow you to have a look at the early versions, I have now published the source module and the built userscript on a separate branch (without a README entry).

ROpdebee commented 3 years ago

Cool! Looking forward to seeing its release :)

On topic: I've spent the day experimenting with webpack as a bundler for userscripts and babel to transpile, the results are here in case you're interested. It works, but the results aren't optimal, in my opinion. This example is fine, but this one is fairly horrible with the babel helpers and some core-js polyfills. I can live with the babel helpers if necessary, but those hundreds of lines of polyfills aren't ideal. I want them minified, if possible. Fortunately, MB already loads most of the core-js polyfills itself, so they won't have to be imported, but some very cool polyfills aren't imported by MB (like these awesome collection methods that I really really want). It's also not possible to just @require them, as they're not distributed that way and I'd have to build and provide those myself (although possible, it's something I'd rather avoid).

I spent the better part of the evening trying to figure out how to tell webpack to minify only external modules, but keep my own modules intact. I don't want to minify the whole file, as the file is presented to a user when it's installed, and seeing one huge string of seemingly random characters may scare some people. I managed to get it to minify each external module separately, but that also didn't give the desired results.

Long story short, I've decided to ditch webpack for the time being, and to try rollup instead. It looks like rollup will provide what I need (still have to verify that, though), and overall it seems that userscript support in rollup is a bit better (ViolentMonkey's generator uses rollup too).

ROpdebee commented 3 years ago

After a couple more hours of experimentation, rollup doesn't provide what I need out of the box, but I think I've found a solution. Experimental code here, in desperate need of a clean up though. The example script now looks exactly like what I wanted: My code is (nearly) intact (apart from babel transpilations), and all of the external dependencies are minified on one line.

The "trick" (more of a hack, IMO) was to bundle the code in two passes. The first pass compiles to two temporary ES modules, one bundled from my code, into tmp_build/${userscriptName}/index.js, and one bundled from the required dependencies, into tmp_build/${userscriptName}/vendor.js. Only the vendor module is minified. A subsequent pass then bundles the two modules into one IIFE into dist/${userscriptName}.user.js.

This probably would've been possible with webpack too, but IMO rollup provides a much simpler interface to hook into the bundling process than webpack. Moreover, the way rollup bundles these scripts looks more natural to the eye than webpack's approach, which hopefully means users will be less scared of the code.

There's still a lot of work to be done, but this looks feasible and promising :smile:

kellnerd commented 3 years ago

the way rollup bundles these scripts looks more natural to the eye than webpack's approach

This is exactly the reason why I decided to go with rollup, besides the native support of ES modules.

When you have your userscripts up and running with metadata (I use rollup's output.banner option for that), Babel and GitHub CI, I will probably draw some inspiration from that to set up Babel and CI as well 😇

ROpdebee commented 3 years ago

I've got the build more or less set up, but it'll take a while to fully launch because I want to migrate all the scripts to the new build system first. That probably means integrating more plugins into the build, maybe switching to TypeScript, setting up JSX (but without React), etc. Ideally I'd want to set up actual tests in CI as well (I think loujine has some cool tests to take inspiration from), perhaps even automated deployments (either by having a GitHub Action create a commit, or an actual tagged release with the built bundles as assets). I still have to figure out whether it'd be possible to switch over to new download and update URLs without causing too much problems for people who have already installed the scripts.

In case you're interested, the build script is rollup.js in the rollup branch, you'll also need the rollup/plugin-userscript.js file. That plugin emits the .meta.js file and adds the header to the .user.js in a way similar to what the banner plugin does. I decided to go with a custom solution since I want the files containing userscript-specific metadata to be ES modules so I can actually execute some code in there, just in case, and AFAIK the existing userscript plugins for rollup only support JSON. It tries to take repo-level information from the package.json, but it's crafted fairly specifically to my own package.json. Overall I think it shouldn't be too difficult to understand what is going on, or to incorporate into your builds. Let me know if there's anything that should be documented better!

On another note, I made a small change to the minification which means that any external imports use a mangled name. I would've preferred it not to do that, but it did lead to a significantly smaller minified version, so it's a small price to pay.

ROpdebee commented 2 years ago

Been doing some digging into automated E2E browser tests, here's a summary of my primary idea so far:

  1. Use playwright to do full automated E2E tests on the latest browser/userscript engine combinations. Test matrix might include Chromium and Firefox, and Tampermonkey, Violentmonkey, and possibly Greasemonkey. Installing extensions into Chrome through playwright should be possible, for Firefox I'm not entirely sure... I'd have to experiment. If FF extensions don't work, we might need to go entirely with Selenium 😨 playwright also supports Safari but I'm not sure whether you can even use userscripts on Safari.
  2. Use Selenium to do some very basic E2E tests on older/other browsers. With "very basic" I really mean exercising one basic interaction pattern just to see whether it works properly (maybe two depending on the scale of the userscript). Reasoning is that since we're transpiling using babel to support older browsers, IMO it makes sense to do a basic test to make sure that the transpilation actually does what it's supposed to do. We might as well also test on Opera/Brave/… to very superficially make sure that nothing goes awry there either. Unfortunately playwright only supports the latest versions of FF/Chrome, so it seems Selenium is the only option to target the older builds. I'd like to avoid Selenium as much as possible though, it's an absolute pain to use.

I'm not sure how resource-intensive these tests are going to get, so a possible option would be to only run a single browser/engine combination (e.g. only Chromium/TM) on PRs, and run the full E2E test suite on merges to main before the scripts are deployed. If an error pops up there, the deployment would get skipped and the bot could file an issue instead, just so that the failure doesn't get lost.