Skip to content

feat: add docker container.#73

Merged
pantierra merged 2 commits into
mainfrom
feat/dockerfile
Jul 23, 2026
Merged

feat: add docker container.#73
pantierra merged 2 commits into
mainfrom
feat/dockerfile

Conversation

@pantierra

@pantierra pantierra commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

@pantierra
pantierra marked this pull request as ready for review July 16, 2026 18:30
@pantierra
pantierra requested a review from danielfdsilva July 16, 2026 18:30
@danielfdsilva danielfdsilva mentioned this pull request Jul 21, 2026
Open
@danielfdsilva
Derive base URL config from a single VITE_BASE_URL var
aa51644 
Vite's asset `base`, the router's basename, Docker's nginx rewrite/<base>
tag, and the GitHub Pages 404 redirect were each configured through
overlapping, duplicated vars (VITE_BASE_URL, VITE_PATH_PREFIX, a
hardcoded vite.config.mts base, and a hardcoded 404.html segment count)
that had to be kept in sync by hand and didn't actually agree with each
other — building with a path prefix and serving the result locally
didn't work.

VITE_BASE_URL (BASE_URL in Docker) is now the single source of truth;
everything else derives from it. Also replaces Docker's separate
config.js file with an inline <!-- __APP_CONFIG__ --> splice directly
into index.html.
@danielfdsilva

danielfdsilva commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

@pantierra Tweaked some things which I pushed directly here.

There were several places where the url was needed and there were multiple configurations.
If built with a path prefix and run locally (without docker) it was not working properly.

The vite config's base was set to ./ but it should also be a prefix, not a local pointer.
The 404 also had problems if there were multiple nested paths.
Also the config file that docker wrote to was ./config which was not processed by vite as a path and would break if accessed from anywhere that's not the root. Likely not a problem with SPA, but not the cleanest approach. Docker now prints the app config directly in the html file. With this we also avoid loading an additional file.

There were also different variables for the base url and the prefix. I figure they could be merged - we provide the full url which is needed for the app, might as well extract the prefix from it.

Please have a look and let me think if you agree with the changes.

@pantierra

pantierra commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

I agree with your take. Looks good to me. This fixes a bug, is much easier and streamlined and the Docker flexibility is there.

@pantierra
pantierra merged commit b285956 into main Jul 23, 2026
4 checks passed
@pantierra
pantierra deleted the feat/dockerfile branch July 23, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danielfdsilva danielfdsilva danielfdsilva approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pantierra @danielfdsilva