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

JS destructuring assignment minification bug #9132

Closed
apokaliptis opened this issue Nov 5, 2021 · 6 comments · Fixed by #9382
Closed

JS destructuring assignment minification bug #9132

apokaliptis opened this issue Nov 5, 2021 · 6 comments · Fixed by #9382

Comments

@apokaliptis
Copy link

Hi, I noticed that Hugo has a problem minifying my javascript. This is my pipeline:

{{ $jspath := "js/main.ts" }}
{{- $jscratch := newScratch -}}

{{ if eq hugo.Environment "production" -}}
  {{ $jscratch.Set "mainjs" (resources.Get $jspath | js.Build | minify | fingerprint) -}}
{{- else -}}
  {{ $jscratch.Set "mainjs" (resources.Get $jspath | js.Build | fingerprint) -}}
{{- end -}}

{{ $mainjs := $jscratch.Get "mainjs" -}}

<script src="{{ $mainjs.RelPermalink }}" integrity="{{ $mainjs.Data.Integrity }}" defer></script>

It worked fine in development using hugo server but when I built for production using hugo --minify I would get an error about an unexpected colon, which means the problem is with Hugo's built-in minification function

I traced it to one of Splide's (v3.2.5) source files which I'm importing like so:

import Splide from '@splidejs/splide';

The source file that Hugo's minifier is having an issue with (after being bundled by esbuild) is @splidejs/splide/src/js/utils/math/math/math.ts, which contains:

export const { min, max, floor, ceil, abs } = Math;

Apparently, after running through esbuild and Hugo's minifier it results in the following summarized code, which is invalid:

var S, O, az, H, g // These are declared with a ton of other variables
// . . . Many characters later . . .
{min:S,max:O,floor:az,ceil:H,abs:g}=Math; // And this where the error occurs.

I was able to get around this by letting esbuild perform the minification instead (which I didn't know you could do at the time of discovering this error):

  {{ $jscratch.Set "mainjs" (resources.Get $jspath | js.Build (dict "minify" true) | fingerprint) -}}

Which is probably the right way to minify my JS in this situation, but I decided to report this as a bug anyway because Hugo's minify tool is taking valid JS from esbuild and returning invalid JS.

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

$ hugo version
hugo v0.88.1-5BC54738+extended windows/amd64 BuildDate=2021-09-04T09:39:19Z VendorInfo=gohugoio

Does this issue reproduce with the latest release?

Yes.

@jmooring
Copy link
Member

jmooring commented Nov 5, 2021

Please generate an un-minified JS file for testing. The result of:

{{ $jscratch.Set "mainjs" (resources.Get $jspath | js.Build) }}

Then post somewhere. This is probably upstream, and we need the "before" state.

@apokaliptis
Copy link
Author

Please generate an un-minified JS file for testing. The result of:

{{ $jscratch.Set "mainjs" (resources.Get $jspath | js.Build) }}

https://gist.github.com/apokaliptis/a715f5113bfba7dc4d0cd5b73b5f454f

@jmooring
Copy link
Member

jmooring commented Nov 5, 2021

Here's a minimal failing example:

var a = 1;
function f() {
  return 1;
}
var { min, max } = Math;
function g() {
  return 2;
}

https://github.com/tdewolff/minify (BAD)

var a=1,min,max;function f(){return 1}{min,max}=Math;function g(){return 2}

https://jscompress.com (GOOD)

var a=1;function f(){return 1}var{min:min,max:max}=Math;function g(){return 2}

@jmooring
Copy link
Member

jmooring commented Nov 6, 2021

Upstream tdewolff/minify#445

@bep
Copy link
Member

bep commented Nov 6, 2021

It's a bug, alright. That said, I would recommend that you use the minify option in js.Build instead, e.g.

{{ $params := (dict "is_production" hugo.IsProduction ) }}
{{ $sourceMap := cond hugo.IsProduction "" "inline" }}
{{ $opts := dict "sourceMap" $sourceMap "minify" hugo.IsProduction "target" "es2018" "params" $params }}
{{ $js = $js | js.Build $opts }}

@github-actions
Copy link

github-actions bot commented Mar 9, 2022

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 Mar 9, 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