ge3224 commented 3 weeks ago

Helper Function for Vite Integration

Overview

This PR introduces a new helper function HTMLFragment that generates the necessary HTML for Vite integration, allowing users to easily incorporate Vite assets into their own HTML templates.

New Feature: HTMLFragment

The new HTMLFragment function has the following signature:

func HTMLFragment(config Config) (template.HTML, error)

This function generates an HTML fragment containing all required Vite assets and scripts based on the provided configuration. The returned fragment is intended to be inserted into the <head> section of an HTML document.

Usage Example

mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
  // viteFragment generates the necessary HTML for Vite integration.
  //
  // It calls vite.HTMLFragment with a Config struct to create an HTML fragment
  // that includes all required Vite assets and scripts. This fragment is intended
  // to be inserted into the <head> section of the HTML document.
  viteFragment, err := vite.HTMLFragment(vite.Config{
    FS:      viteFS,
    IsDev:   *isDev,
    ViteURL: "http://localhost:5173",
  })
  if err != nil {
    panic(err)
  }

  tmpl, err := template.New("index.tmpl").ParseFS(goIndex, "index.tmpl")

  if err != nil {
    panic(err)
  }

  if err = tmpl.Execute(w, map[string]interface{}{
    "Vite": viteFragment,
  }); err != nil {
    panic(err)
  }
})

In the HTML template:

<head>
  <meta charset="UTF-8" />
  <title>My Go Application</title>
  {{ .Vite }}
</head>

An example implementation is included in the PR for testing purposes.

Note

I apologize for submitting this PR before @danclaytondev had a chance to contribute their implementation. I was already working on this and had some time today to prototype it. I'm more than happy to collaborate or step back if @danclaytondev wishes to take the lead on this feature.

olivere commented 2 weeks ago

Does the helper work if we have multiple entry points? For me, it's a non-issue, but I think @ge3224 Jacob had a use case for that.

danclaytondev commented 2 weeks ago

Does the helper work if we have multiple entry points? For me, it's a non-issue, but I think @ge3224 Jacob had a use case for that.

I believe so, we have the ViteEntry field on the config struct, which then is used to extract chunks from the manifest. So if you have multiple entrypoints, you can use a new vite.Config on each entrypoint and generate a new fragment for each.

olivere commented 2 weeks ago

Does the helper work if we have multiple entry points? For me, it's a non-issue, but I think @ge3224 Jacob had a use case for that.

I believe so, we have the ViteEntry field on the config struct, which then is used to extract chunks from the manifest. So if you have multiple entrypoints, you can use a new vite.Config on each entrypoint and generate a new fragment for each.

Yes, of course. Sorry for the noise. That makes perfectly sense, and I really like that approach: One instance per entry point.

olivere commented 2 weeks ago

@ge3224 @danclaytondev I've finally had the time to test this myself. And I like it. I've added another example in examples/router. That's a "full-stack" React app with client-side routing (with TanStack Router), server-side routing (refreshing a page makes the server delegate to the frontend), API handling (with TanStack Query), and serving public assets. Looks quite solid for our first attempt.

Besides the examples/router application, I've changed two things:

Add a config.ViteManifest option that defaults to .vite/manifest.json. If specified, it instructs the library to lookup the manifest from that specified file.
The ViteURL default of http://localhost:5173 wasn't set when using the vite.HTMLFragment function. Now it does. I think we should have centralized defaults handling for both NewHandler and HTMLFragment, but it works for now.

Feel free to look at it again.

Is there anything that we're still missing? If not, maybe we can merge this in the next few days. What do you think?

danclaytondev commented 2 weeks ago

I think this is looking really good! I have also been doing some testing over the last couple of days and like it a lot. I think you've already resolved a lot of the stuff I have found.

The only thing we haven't discussed fully yet is serving files in dev mode. I think we all know/agree that we don't want to serve js/css in dev mode, Vite does that for us and lets us use HMR.

What I found was some initial issues with files in the public folder, and simple assets like svg that are complied/included by Vite. For files imported by Vite (i.e. via javascript import statements) they are given a relative URL. This means that they are sent to our Go server rather than the Vite server.

I think @olivere has also found this, so is serving the public folder in dev mode. I think that's a good option. Personally I would disable the Vite public folder, and serve those assets from another folder not using Vite. Just plain Go.

In summary, I think this PR is ready to go. I think we should handle serving static assets with Go as much as possible, and concentrate on being a helper function for the script tags that we need for Vite, rather than trying to serve assets. There are so many different ways and patterns of serving assets that we should leave that to individual devs and just give them the script tags for js/css. If they want to use images with Vite etc that is fine, we can let them do that but they need to serve the assets from their setup.

The only thing I would add is some more info in the readme about serving assets in dev mode. That can come later depending on what you guys think.

I will also add some tests to the code in this PR, once we have all agreed on the functionality. Maybe after its merged.

Thanks for your hard work. :)

ge3224 commented 2 weeks ago

@olivere Thank you for your routing example. I've made a minor adjustment to address an additional dependency requirement. Apart from this small change, I believe we're ready to merge.

I appreciate both @olivere and @danclaytondev for highlighting several areas where we could introduce configuration options. While we may not have anticipated every scenario, as @olivere mentioned, this is our initial attempt. We can always incorporate additional features as needs arise.

@danclaytondev Your suggestion for unit tests is excellent.

If there are no further concerns, shall we proceed with the merge?

olivere commented 2 weeks ago

Oh, sorry, I missed that dependency...

I'm happy to Squash and merge (to get a clean history). If there is no veto or late change, I will do so over the coming days.

Thanks so much for working on this. It's been a pleasure to work with you. @ge3224 @danclaytondev

danclaytondev commented 2 weeks ago

Oh, sorry, I missed that dependency...

I'm happy to Squash and merge (to get a clean history). If there is no veto or late change, I will do so over the coming days.

Thanks so much for working on this. It's been a pleasure to work with you. @ge3224 @danclaytondev

Sounds great @olivere. I have some documentation changes to suggest, but I think it would be more productive to merge this (either main or maybe a feature branch) then I can open a PR, rather than trying to do more in this PR. Functionality wise this is looking great.

ge3224 commented 2 weeks ago

Oh, sorry, I missed that dependency...

I'm happy to Squash and merge (to get a clean history). If there is no veto or late change, I will do so over the coming days.

Thanks so much for working on this. It's been a pleasure to work with you. @ge3224 @danclaytondev

@olivere By all means, feel free to squash as you see fit.

Also, I've been enjoying working with both of you, @olivere and @danclaytondev.

olivere / vite

feat: add helper func for minimal integration #10

Helper Function for Vite Integration

Overview

New Feature: HTMLFragment

Usage Example

Note