Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in media type suffix lookup #8406

Closed
cmahnke opened this issue Apr 10, 2021 · 16 comments · Fixed by #8444
Closed

Regression in media type suffix lookup #8406

cmahnke opened this issue Apr 10, 2021 · 16 comments · Fixed by #8444
Assignees
Milestone

Comments

@cmahnke
Copy link

cmahnke commented Apr 10, 2021

What version of Hugo are you using (hugo version)?

$ hugo version
+hugo v0.82.0+extended darwin/amd64 BuildDate=unknown

Does this issue reproduce with the latest release?

Yes

Description

I've defined a custom output format to generate GeoJSON:

[mediaTypes]
  [mediaTypes."application/geo+json"]
    suffixes = ["geojson", "gjson"]

[outputFormats]
  [outputFormats.GEOJSON]
    mediaType = "application/geo+json"
    isPlainText = true
    isHTML = false
    baseName = "index"
    notAlternative = true

When this is added to config.toml, the following error message is displayed during the build:

Error: Error building site: JSBUILD: failed to transform "js/main.js" (application/geo+json): unsupported Media Type: "application/geo+json"

I suspect the error in 'resources/resource_transformers/js/options.go', line 319, somehow the media type get's passed there - I don't really understand why.
If it's just because of the application/ prefix of the media type some XML formats (application/xml) will suffer as well.

I've updated the example repository from #8370, see cmahnke/hugo-0.82-test

@cmahnke
Copy link
Author

cmahnke commented Apr 11, 2021

Changing the mediaType from application/geo+json to application/json fix this issue, but it's certainly not how it should work, since it isn't documented.

A mid term solution would be a flag like isHTML. It would be called isJS and would be evaluated in resources/resource_transformers/js/options.go, if set to false the media type would be simply ignored there.

@matt9189
Copy link

The mediaType is passed to esbuild in the options to determine which content loader esbuild should choose to process the file. esbuild only supports certain content types and cannot process custom formats in the way that Hugo does. The error message that you received seems like it is the correct way for this to be handled, indicating that you are attempting to use js.Build with an unsupported media type and so it does not know how to process the data.

@cmahnke
Copy link
Author

cmahnke commented Apr 18, 2021

I'm sorry but I can't see where I pass it, it seems to be passed implicit...
Cann you please point out the line here:
https://github.com/cmahnke/hugo-0.82-test/blob/main/layouts/partials/head.html

@matt9189
Copy link

Going through your test program (which I did not see before, I apologize), I was wondering if either of the two js dependencies have an import statement that imports a geo+json file? Does Mirador have an import for that file type within its code?

@matt9189
Copy link

It looks like openlayers defines a geoJSON type and imports it within the code. As js.Build goes through the files to create a single javascript file, it must attempt to import this file, does not recognize the type, and this is likely where the failure is occurring. I think that is the case, though I do not know how to address it.

@cmahnke
Copy link
Author

cmahnke commented Apr 18, 2021

Well, I've just updated my test case since there might have been the case that OpenLayers has to do something with this. But even without OpenLayers (or Mirador) this happens. It's a hugo issue (not ESBuild), since the error is risen here

@matt9189
Copy link

matt9189 commented Apr 18, 2021

When the custom media type is created, it creates a suffixesCSV which contains "geojson, gson." As Hugo processes the mediaTypes for javascript files, it checks to see the associated media types based on the suffix "js."

hugo/media/mediaType.go

Lines 307 to 308 in 33d5f80

func (m Type) hasSuffix(suffix string) bool {
return strings.Contains(m.suffixesCSV, suffix)

This code sees "js" in "geoJSon" and adds it as the first mediaType of javascript files which then passes it to the js.Build and creates the error. This is a bug.

Edit: it is actually occuring when a new resource is created here:

mimeType, suffixInfo, found := r.MediaTypes.GetFirstBySuffix(strings.TrimPrefix(ext, "."))

Though, the issue will occur at the other location also.

I'm not sure where the custom resources are added to the spec mediaTypes, but if they are added after the built-in types it could address the problem temporarily as the custom type would not be the first associated type.

@cmahnke
Copy link
Author

cmahnke commented Apr 18, 2021

@matt9189 Thanks for investigating - since there is a workaround and only a few affected suffixes I guess it has a lower priority.

@matt9189
Copy link

I saw during my debugging that there was a "todo" regarding the ambiguous nature of mediaTypes in resource_spec.go. The above issue may be fixed by checking for an exact match in the suffixesCSV. I am not going to create a pull request, mostly because I am not very fluent at using github and I need someone to show me some stuff. I will post this here though as a possible fix for mediaType.go hasSuffix() function above.

mimeType, suffixInfo, found := r.MediaTypes.GetFirstBySuffix(strings.TrimPrefix(ext, "."))
// TODO(bep) we need to handle these ambiguous types better, but in this context
// we most likely want the application/xml type.
if suffixInfo.Suffix == "xml" && mimeType.SubType == "rss" {
mimeType, found = r.MediaTypes.GetByType("application/xml")
}

func (m Type) hasSuffix(suffix string) bool {
	for _, s := range strings.Split(m.suffixesCSV, m.Delimiter) {
		if strings.EqualFold(suffix, s) {
			return true
		}
	}
	return false
}

@bep
Copy link
Member

bep commented Apr 19, 2021

Why do you pass these files into js.Build?

@cmahnke
Copy link
Author

cmahnke commented Apr 19, 2021

@bep: I don't pass anything - js.Build breaks if there is just such a definition in config.toml - the example doesn't even contain templates for this...
This makes it a bug, not an enhancement ;)

@bep
Copy link
Member

bep commented Apr 19, 2021

@cmahnke

  1. The only way I see that error happen is if you pass a resource of MIME type application/geo+json to js.Build
  2. I tested the GitHub project above, and I don't see any error

@cmahnke
Copy link
Author

cmahnke commented Apr 19, 2021

@beb: Maybe your using a development version of Hugo? Or am I missing something by reusing the #8370 test case?
Here is the problem with the example repository: https://github.com/cmahnke/hugo-0.82-test/runs/2383428063

@bep
Copy link
Member

bep commented Apr 20, 2021

@cmahnke no, I forgot to init the submodule when I cloned it. I get the error now, I will eventually get to it.

@bep bep added Bug and removed Enhancement labels Apr 20, 2021
@bep bep changed the title Custom output format breaks jsbuild Regression in media type suffix lookup Apr 20, 2021
@bep bep added this to the v0.82.1 milestone Apr 20, 2021
@bep bep self-assigned this Apr 20, 2021
bep added a commit to bep/hugo that referenced this issue Apr 20, 2021
Introduced in Hugo 0.82.0.

Fixes gohugoio#8406
@bep bep closed this as completed in #8444 Apr 20, 2021
bep added a commit that referenced this issue Apr 20, 2021
Introduced in Hugo 0.82.0.

Fixes #8406
bep added a commit that referenced this issue Apr 20, 2021
Introduced in Hugo 0.82.0.

Fixes #8406
@cmahnke
Copy link
Author

cmahnke commented Apr 20, 2021

@bep: Thanks for fixing!
From my point of view there is no need to create a new release 0.82.1 just for this since I now use the described workaround.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants