Adamcake / Bolt

An alternative launcher for your favourite MMO
GNU Affero General Public License v3.0
122 stars 18 forks source link

Refactor Svelte Configs #47

Closed kmcgurty closed 2 weeks ago

kmcgurty commented 2 weeks ago

Hello - great project you have going on here! I was looking through the source code and noticed you're using Svelte. I've been using Svelte in a production environment for over a year a this point. I'd like to help refactor some of the code base to help take advantage of Svelte's features. This PR will lay out the foundation to help accomplish that, while making the build experience more approachable.

This PR doesn't change any of the functionality of the front end, it just changes around the Svelte configs, some formatting changes, and updates the readme to reflect all of these changes.


If you've previously ran bun install, you'll have to run it again as I've added new packages. There's ways to automate installing on pull, but that's outside the scope of this PR. I have some other thoughts on the actual build process, but that's also outside the scope of this PR. I'll create an issue at some point with all of the suggested changes to the Svelte code. Please let me know if I've missed anything.

kmcgurty commented 2 weeks ago

It has just occurred to me that the root readme.md is out of date instructing to build tailwind. Also the table of contents is wrong in app/readme.md. I can update those later today.

Adamcake commented 2 weeks ago

Tagging @smithcol11 as he made most of the Svelte UI.

Thanks - I can look through this later but it doesn't look too substantial, which is good.

I noticed you've made .prettierignore not end in a newline in fe6bed4. Text files should always end with a newline. The main readme mentions using an editor which respects editorconfig, which would've fixed this for you when you saved the file.

There's ways to automate installing on pull, but that's outside the scope of this PR.

I don't think I would want that, since not everyone who clones this repository is looking to work on the UI. Strictly speaking you can build Bolt without having a JS runtime installed at all.

kmcgurty commented 2 weeks ago

Good catch - fixed in 1f724c5. I've added the EditorConfig extension to the recommended list inside app/.vscode/extensions.json. I initially thought it'd be a problem since I've only been opening the app/ folder in vscode, but it apparently traverses up until it finds an EditorConfig file.

Edit: I realized this might be missing some context if you're not familiar with .vscode. This will prompt a developer to install this extension when they open the app/ folder in vscode.

You might be able to get fancy with github workflows that make it so PRs can't be committed if they fail certain checks, but I've never set that up before. Might be worth looking into if it continues to be a problem.

Adamcake commented 2 weeks ago

Coincidentally, I learned today that the repo for the Zig programming language has it set up so that a PR will fail if zig fmt would lead to any changes. Apparently it's based on git bisect. I can think of a few ways this would be useful for Bolt - editorconfig for the repo, prettier for app/ sources, and bun run minify for app/dist, in that order.

But I think that's a problem for another day.

Adamcake commented 2 weeks ago

Seems fine, thanks!

smithcol11 commented 2 weeks ago

Just wanted to mention that these changes look great to me, nice job.

@kmcgurty Svelte was new to me with this project, so I may not have used its features or standards to the fullest extent. Let me know if you have any suggestions or want to collaborate on it sometime! :smiley: