refactor(deezer_importer): convert userscript to TypeScript - #1023
Conversation
4853486 to
a45b160
Compare
|
That failed check because of missing empty line was devastating |
|
Damn, I forgot I did another improvement while refactoring. The "original mix" cleaning was not working on releases where it was included directly in Also removed the ISRC Hunt button that probably sneaked in because my own Deezer script has it. |
arsinclair
left a comment
arsinclair
left a comment
There was a problem hiding this comment.
Sorry, took me a while to actually get to it, busy week.
I have a few comments about changes, please take a look below.
I would like to have a complete parity with the old script: you can add correct types, but please keep the bug fixes and new features to other PRs. In the future they can be merged fairly quickly, while currently they pollute the scope of the PR making it harder to review and require more testing. I'm seeing at least two additions which are not part of the original codebase: notable Harmony button and barcode badge. Same about tests, it's yet another thing to review. All of these additions are good additions, but please understand where I'm coming from: if I see a PR that takes 10 minutes to review, I might squeeze it and get it checked the same day. If I see a PR that will likely take a couple of hours to review, it might take a while before I find the slot. Scope creep is a real issue here.
2e90f8f to
8c82f6a
Compare
|
Sigh, I knew this would be getting though. Hopefully everything in order now. Checked manually with existing release: |
|
Thanks for a quick fix. Could you please also add a console.warning message, something similar to and add deprecated info to the script name and description, similar tomusicbrainz-userscripts/cd1d_importer.user.js Lines 2 to 3 in 2e5132d This is an extra safery measure if for some reason clients cannot upgrade to the TS version (it's theoretically possible as the distribution URL change might look suspicious enough for some security-oriented contexts to block it). I'll try to take another look at the PR today. |
* Migrate userscript to modular TypeScript under src/userscripts/deezer_importer/ * Add strict types matching Deezer API entities and response shapes * Update root deezer_importer.user.js to redirect downloads and updates to dist/ * Maintain 1:1 functional parity with legacy userscript logic
fad3e1f to
22249da
Compare
arsinclair
left a comment
arsinclair
left a comment
There was a problem hiding this comment.
I finally had time to go through all the code and left some comments along the lines.
Could I ask you to be more involved in the process instead of letting AI drive it? It is possible to rewrite all scripts to TS in less than 10 minutes using AI, but I think human attention to detail is equally important.
* Restore raw track duration in milliseconds without rounding * Inline track title formatting and remove unused helper function * Remove unused barcode property from ParsedDeezerRelease return * Restore legacy joinphrase calculation using original contributor array indices * Remove speculative artist fallback from parseDeezerRelease * Align release URL and album ID extraction inside initialization timeout * Move parseDeezerRelease invocation from insertLink to initialization flow * Append toolbar items directly to toolbar element without desktop wrapper div * Restore mobile mounting explanatory comment * Update internal import paths to use established ~ alias * Update userscript icon to vector SVG image * Bump script versions past upstream 2026.8.26 releases for auto-upgrade
…efactor/typescript-migration
|
Last try. If it still sucks I'll delete this. Clearly this exceeds my capabilities. Sorry for having wasted your time. |
|
It doesn't suck, most of the things I'm asking about is for a single specific reason: traceability. We should be able to use git history in the future to see what changed and why. But if functional changes or bug fixes are combined within a TS rewrite such changes practically become invisible since there's no 1-to-1 correlation between the code and you don't see the logic changes it in the diff. Regarding AI: it's fine in some of my comments where I'm clear that something must be changed to just change it. But in other comments I'm asking a question: why something was done a certain way I think it is reasonable to expect that you should be able to explain it, rather than change it straight away and resolve the comment. I'm not trying to challenge you here but rather I'm also trying to understand the change. Hope it makes sense 😉 |
|
I hope you don't mind the AI answers to your questions. I can't explain it as technically (or don't know why the heck it did that). Maybe I need to pay more attention, or need better prompts, or other tools that can detect this. |
|
I haven't fully formed an opinion about AI answers yet. But I guess I gravitate more towards the fact that I can ask AI too, so human rensponses is better. Antigravity? Man, you're doing yourself a disservice :) Google models currently are one of the worst ones on the market. Maybe you have a very valid reason to use it, I don't know, but if not, I really suggest using GPT 5.6 Sol (Medium). It's a beast and it's cheap. My current setup is VSCode with a Codex extension, a Plus plan with OpenAI on GPT 5.6 Sol (Medium), which performs amazing at most of the tasks. My second setup is a Cursor, mainly for their Composer 2.5 model, which is lightning fast. I use it mostly for analysis and gathering and assembling context from multiple sources, although I can see that it's quite good at reasoning too for mid-level tasks. |
Understandable but if you ask AI yourself you lack the context (in this case) 😉
Yes, mainly because I got it 1 for free for a full year and next year is probably free as well. 🙂 Footnotes
|
|
I was kinda referring to this: https://gruhn.me/blog/2026-08-03 (although they are quite harsh with words). |
|
It's very direct but right. There is no point in having another human in the loop when reviewers could use AI themselves. |
My take on this is optimistic though: I don't think the article calls to eliminate the human from the loop but rather urges the human to take the control back. Off topic aside, would you like to submit your other improvements to the script now that I asked you initially to separate? |
On it. Doing navigation fix only for now: #1070 I have also found out my own repo's guides etc. were spilling over to other repos because I had them all open in same VS Code workspace for ease of use. 🤦 Hopefully less unintended code changes by AI I have to deal with now. |
|
Ah, good that you figured it out! |
2 participants
Note: Code and description were written with AI-tools
Description
Converts the
deezer_importeruserscript to modular TypeScript undersrc/userscripts/deezer_importer/, providing full static typing while maintaining strict 1:1 parity with the legacy userscript.Key Changes
index.ts,meta.json,types.ts,getDeezerReleaseData.ts,parseDeezerRelease.ts).deezer_importer.user.jsmetadata (v2026.05.31.2) to redirect downloads and updates todist/deezer_importer.user.js.waitForEl), track pagination, and error abort behavior.Verification
pnpm type-check(tsgo),oxlint --type-aware, andeslint .pass with 0 errors.oxfmt --check.dist/deezer_importer.user.js.