feat: add frontend build #350
Conversation
| @@ -17,6 +17,9 @@ import './App.css'; | |||
| import { useEffect, useState } from 'react'; | |||
| import { Icon } from '@launchpad-ui/icons'; | |||
|
|
|||
There was a problem hiding this comment.
We use paths like /api/dev/projects/:projectKey in local dev, because the /api prefix triggers proxy-matching behavior in the frontend devserver to forward the requests to the API service.
We want paths like /dev/projects/:projectKey in the published CLI, because the API server is also serving the frontend. Toggle this behavior using meta data from the Vite build.
| const plugins: PluginOption[] = [react(), svgr()]; | ||
|
|
||
| if (command === 'build') { | ||
| plugins.push(viteSingleFile()); |
There was a problem hiding this comment.
Inline everything into a single HTML when running a production build, for the local dev loop just use the defaults.
| @@ -5,7 +5,7 @@ import ( | |||
| "net/http" | |||
| ) | |||
|
|
|||
| //go:embed index.html | |||
| //go:embed dist/index.html | |||
There was a problem hiding this comment.
@mike-zorn the output of Vite by default is {repo_root}/internal/dev_server/ui/dist/* because we run the build command from {repo_root}/internal/dev_server/ui... as a result I had to change this embed pattern, and when browsing to localhost:8765/ui I see a list of filesystem contents (guessing this is the static file serving middleware?)
..browsing to localhost:8765/ui/dist results in the {repo_root}/internal/dev_server/ui/dist/index.html file being served as expected, but I don't think we want the "dist" portion of the path? Not a big deal, just noting for posterity
There was a problem hiding this comment.
We also made the decision not to introduce the frontend build into the build pipeline at this point, because we'll need to update the GH Action to use node/npm to run npm run build.
...for now, let's just run npm run build locally and check in the artifact.
There was a problem hiding this comment.
Took a look. I think I see how to fix it. I'll add a commit to the PR.
There was a problem hiding this comment.
Pulled the change down and tested, looks good. Thanks! 🙌
kwatkins-ld
force-pushed
the
kwatkins/frontend-build
branch
from
2d91a87
to
c10b9d4
Compare
Requirements
Adding a frontend build for the devserver
Related issues
Provide links to any issues in this repository or elsewhere relating to this pull request.
Describe the solution you've provided
Provide a clear and concise description of what you expect to happen.
Describe alternatives you've considered
Provide a clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context about the pull request here.