Fix assets helpers

[stanislas-m]

• Fix Assets helpers are not CDN friendly #921: use webpack publicPath, instead of hard-coded "/assets/".

[markbates]

@stanislas-m is this backward compatible?

[stanislas-m]

@markbates I was thinking about patching the webpack config, using the update command. Just need to test for the publicPath property in the config output, and it should work fine.

The non-webpack scenario works the same as before.

So, to answer the question, right now, it's not.

[markbates]

This needs to be backward compatible. Either compatible on the go or the js side. Alternatively it should be fixed, or a warning telling people how to fix it, with buffalo update.

[stanislas-m]

@markbates I've added a new warning in the update command for this case. I can see two cases:

• The user doesn't know how to configure webpack, and choose to override the config to the last one. => ok
• The user changed the webpack config. There are many ways to change the file, so patching it is a bit difficult. => show a warning, with a link to webpack docs.

I've also added a way to set the assets root using the ASSET_PATH env var.

[markbates]

@stanislas-m after reviewing this, I'm concerned about the backward compatibility issue with this PR. I know there's a warning, but if people don't run buffalo update or, as happens often they ignore warnings, it's going to result it a lot of issues and help requests.

I believe I understand the idea, allow customization of the asset pipeline from the asset pipeline, i.e. webpack.config.js.

In order to pull this in, without backward compatibility, we'd have to do a v0.12.0 release, which I don't think we should do right now.

How would you feel about either finding another solution or attaching this to a 0.12.0 milestone?

[stanislas-m]

@markbates Sure. Give me some time, I'll check if I can find a better solution.

[codecov]

Codecov Report

Merging #1030 into development will not change coverage.
The diff coverage is 100%.

@@             Coverage Diff              @@
##           development    #1030   +/-   ##
============================================
  Coverage        60.83%   60.83%           
============================================
  Files               58       58           
  Lines             2962     2962           
============================================
  Hits              1802     1802           
  Misses            1046     1046           
  Partials           114      114

Impacted Files	Coverage Δ
render/template_helpers.go	95.08% <100%> (ø)	⬆️
middleware/tokenauth/tokenauth.go	73.17% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6f761b...4b5f599. Read the comment docs.

[bittopaz]

What is the status of this PR, please? @stanislas-m

I'm running into the same issue while trying to deploy my application to production with CDN.

Or do we have a workaround that I can use to make current buffalo app work well with CDN, please?

[stanislas-m]

@bittopaz It's not ready yet, I'll try to find some time to finish it.

[bittopaz]

Sure, take your time please.

And can we also support a configuration for asset_host, so I can config asset_host for different
env, just like how rails handles this?

[truesilver92]

@bittopaz I think the workaround right now is to avoid the assetPath based functions in your application.plush.html file.

[truesilver92]

@markbates I don't think this issue should be closed. It still is a bug essentially caused by a magic number in the code. I am willing to fix it in a backwords compatable way, but it would not be the correct way to fix this. gobuffalo's version is still 0.x for a reason. Breaking changes could come at any time. Many fixes should be incompatible so that bugs can be fixed correctly instead of being backwords compatible with code based on buggy behavior.

Part of the reason to call it a bug and not a feature request is that this bug prevents correct usage of webpack. You cannot do code splitting/chunking https://webpack.js.org/guides/code-splitting/

[H3rby7]

I'm also running into this issue with the assetsPath. Willing to contribute to fix it. Meenwhile is there a convenient different way to access the asset files? Asking because the file names contain the hash of some sort, so I cannot really "hard-link" them without helpers :/

[truesilver92]

I wrote a bit of a work around which is a bit better in some ways (the loading of the manifest is not blocked by a mutex or syncing between requests for example)

this is essentially my render.go

package actions

import (
	"encoding/json"

	"github.com/gobuffalo/buffalo/render"
	"github.com/gobuffalo/envy"
	"github.com/gobuffalo/packr/v2"
)

var r *render.Engine
var assetsBox = packr.New("public_assets", "../public")
var assetMap map[string]string

func loadManifest() map[string]string {
	manifest, err := assetsBox.FindString("assets/manifest.json")
	if err != nil {
		App().Logger.Warnf("could not read webpack manifest file %v", err)
		return map[string]string{}
	}
	m := map[string]string{}
	err = json.Unmarshal([]byte(manifest), &m)
	if err != nil {
		panic(err)
	}
	return m
}

func assetPath(file string) string {
	m := assetMap
	if Env == "development" {
		m = loadManifest()
	}
	return m[file]
}

func init() {
	assetMap = loadManifest()
	r = render.New(render.Options{
		// HTML layout to be used for all HTML requests:
		HTMLLayout: "application.html",

		// Box containing all of the templates:
		TemplatesBox: packr.New("templates", "../templates"),
		AssetsBox:    assetsBox,

		// Add template helpers here:
		Helpers: render.Helpers{
			"env":         func() string { return app.Env },
			"asset_path":  assetPath,
			// uncomment for non-Bootstrap form helpers:
			// "form":     plush.FormHelper,
			// "form_for": plush.FormForHelper,
		},
	})
}

replace the nicer functions for creating script tags, etc. in your plush files with asset_path as part of the workaround

I also modified my webpack.config.js to set the publicPath to "/assets/"

output: {
   filename: "[name].[hash].js",
   path: `${__dirname}/public/assets`,
   publicPath: "/assets/",
},

[H3rby7]

Uhh, very nice. I went for publicPath: "assets/" without the base /, to get my behavious of serving everything under a prefix :)

Thanks a lot @truesilver92

[jh125486]

Can this be re-opened and revisited?