chore: remove unnecessary env vars #3049
Open
+50
−58
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: John Cowen <[email protected]>
✅ Deploy Preview for kuma-gui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
slonka
reviewed
Oct 21, 2024
| import { defineConfig, mergeConfig, type UserConfigFn, type UserConfig } from 'vite' | ||
|
|
||
| import { config as prodConfig } from './vite.config.production' | ||
| const version = JSON.parse(read('./package.json').toString()).version | ||
| // https://vitejs.dev/config/ | ||
| export const config: UserConfigFn = (env) => { | ||
| return mergeConfig( | ||
| prodConfig(env), |
There was a problem hiding this comment.
unrelated, but why is there a prodConfig in a file called development.ts?
Comment on lines
+20
to
+27
| baseGuiPath: '/gui', | ||
| apiUrl: 'http://localhost:5681', | ||
| version, | ||
| product: 'Kuma', | ||
| mode: 'global', | ||
| environment: 'universal', | ||
| storeType: 'postgres', | ||
| apiReadOnly: false, |
There was a problem hiding this comment.
This seems the same as https://github.com/kumahq/kuma-gui/pull/3049/files#diff-47b7ce02023c49f9552984f4026c55914f09f8d403fd8cc08f875f78cc0d1baeR8-R16 - should we apply DRY on it?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Back in the mists of time we used a plugin in order to interpolate variables into our
index.htmlfile. We need this as in 'production' kuma itself uses ourindex.htmlas a template and interpolates variables into it.Since then vite was upgraded to v5 which included functionality to interpolate variables, but only shell environment variables. This led us to drop the plugin for variable interpolation and use environment variables instead. The only thing is this meant we also started adding single line stringified JSON into environment variables.
This has always sat a little strange to me, these variables don't need to be shell environment variables and the stringified JSON is awkward.
Additionally lately we've realised we want to use a variable from the
package.jsonfile ("version": ...) to prevent duplication of this value in several places. (see #2967)Because of this last point, I decided it was a good time to revisit this.
This PR aims to replicate exactly what the kuma binary does. We use JS to search and replace the
golangstyle{{.BaseGuiPath}}variables with the actual variables. This is achieved using a relatively simple almost single line "inline vite plugin". As we replicate what kuma does, it means we don't have to do anything special anymore for 'production', we just ship the template as is, then only in 'development' we interpolate variables.This also removes the need for the shell environment variables and the awkward stringified single-line JSON, and means we can reference the
package.json#versionvia JS where we need to.Lastly, I removed some fallback JSON that we had for a case that I don't think I've ever seen happen (an un-parseable HTML file), and really, if the HTML is un-parsable we probably don't want anyone using the resulting GUI as its likely to be very broken and/or unadvisable to use.
Overall this PR simplifies things quite a bit.