Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch frontend globals to use Vite defines #509

Merged
merged 3 commits into from
May 17, 2024

Conversation

Gcommer
Copy link
Contributor

@Gcommer Gcommer commented May 17, 2024

I did this refactor for #503 before realizing I could just use window.gbans.site_name

Figured I'd offer this to you anyways as using Vite defines is a bit more idiomatic and compiles a little more efficiently (it saves ~200 bytes off of index.html and ~1kb off of index.js)

I tested by poking around the site and also running ./node_modules/.bin/tsc --noEmit and noticing no new type errors. release.sh also still works.

This PR temporarily is stacked on top of the commit from #503... sorry, I'm not sure about modern github etiquette for stacked commits, I'll sort it out as that PR lands.

@leighmacdonald
Copy link
Owner

Its my first time using vite, so i'm sure i've done a number of things sub optimally :). Thanks for the changes, looks good. I fixed up the minor conflict.

@leighmacdonald leighmacdonald merged commit 5ecd48b into leighmacdonald:master May 17, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants