search

nukeop / nuclear

Streaming music player that finds free music for you
https://nuclear.js.org/
GNU Affero General Public License v3.0
12.02k stars 1.04k forks source link

Converted lyrics components into TypeScript #1584

Closed sicin closed 5 months ago

sicin commented 5 months ago

The PR doesn't change functionality, those are TypeScript changes with some slight tweaks to logic (e.g. LyricsView component lyricsStr creation, return type of lyricsSearchSuccess did not make much sense to me as it was never an array) and naming (lyric -> lyrics in function names and enums)

I guess Nuki-chan might be unhappy package-lock.json has been changed, but since I'm a new contributor who just did a fresh npm install a few days ago, I don't really understand why @nuclear/scanner is not in the dependencies of core. You can verify that I just did npm install and didn't change it directly by creating a fresh package-lock on your machine.

Since it's my first commit here, I welcome any feedback, especially since I'm still unsure what's the current gold standard for some of the functionalities/components. I'm happy to learn, help and hope to continue contributing.

nukeop commented 5 months ago

Hi, everything looks alright. When you're converting existing files to typescript, it's worthwhile to also update some outdated code, like .then to async-await syntax, or Object.assign to spread ({...thing}). But this is also good enough.