Edge case with Uglify and Svelte Code

[ekhaled]

I do not know if anyone has come across this problem before.

It's common to see compiled code like this:

//.... preceding code
p: function update(changed, state) {
  if (current_block_type !== (current_block_type = select_block_type(state))) {
    if_block.u();
    if_block.d();
    if_block = current_block_type(state, component);
    if_block.c();
    if_block.m(if_block_anchor.parentNode, if_block_anchor);
  }
}

And then a bit further down the file, the select_block_type function definition as:

function select_block_type(state) {
  if (state.foo) return create_if_block;
  return create_if_block_1;
}

The problem arises when Uglify inlines the functions returning from select_block_type.
So the Uglify-ed code looks something like this:

function s(a){
   return a.foo ? full_body_of_create_if_block : full_body_of_create_if_block_1
}

Now the inequality check in update always returns true, and leads to some really weird bugs.

The only way around it, I found, is turning off reduce_vars for the uglify plugin, which forces it not to inline functions.
Something like:

...
plugins:[
  svelte(),
  uglify({ compress: { reduce_vars: false } })
]
...

---

I hope the above makes sense.

I don't know if this is the right place to post this, but I thought it might help someone tearing their hair out in the future.

Also, I cannot seem to create a reduced example of when uglify does this. But it seems to do it on large codebases, so I think it might be to do with the amount of code.

If anyone else has come across this, please let me know of your experiences.

[paulocoghi]

I believe Rich Harris can solve this problem on Svelte side, but it would also be interesting to post this bug in Uglify issues (if you have not posted there yet).

[Rich-Harris]

Oh wow! That is curious. I bet @kzc would be interested to see this. I can't reproduce it either, but if you manage to then it would be very helpful to share.

[ekhaled]

I can't reproduce it outside my codebase.
One thing I noticed is it happens at the bottom of the bundle.

Because it is related to reduce_vars I think it might be to do with the number of variables in a given scope (maybe @kzc can confirm this).

I'll try to reproduce it tomorrow by introducing a large number of arbitrary variables.

@paulocoghi I don't know whether to call this a bug lol, I mean both svelte and uglify are doing what they are meant to do.

Having said that relying on strict equality with functions might be something that needs looking at from the svelte side.

[kzc]

Insufficient information to reproduce. If you do, create an issue on the uglify side.

[kzc]

I managed to create a failing test case for uglify out of the description above - although it may not necessarily be the same problem described in this issue. This uglify bug is related to the conditionals compress option - not reduce_vars.

@ekhaled Feel free to post the complete original unminified javascript input to uglify which causes it to produce incorrect code so I can verify whether it's the same issue. The size of the input file does not matter - but it must be the entire program.

[ekhaled]

This REPL inlines functions inside select_block_type after running through buble and then uglify

I think its the same issue as @kzc mentioned, but I will try to create an even more reduced case and post it over there

[kzc]

Only interested in the javascript input to uglify. Presumably that's post-Svelte and post-Bublé.

Can you verify whether or not it works with the following options?

compress: {
    reduce_vars: true, 
    conditionals: false,
},

[ekhaled]

Posted a reduced failing case on the linked issue, let's discuss further over there 😃

[ekhaled]

@Rich-Harris given the simplicity of the reproduction, looks like thousands of components in production might be re-rendering needlessly. I'm just surprised I haven't come across this earlier (or no one else has).
I only found out when I had a bound input inside an else block that kept losing focus on every keydown.

A general announcement might be in order explaining the workaround and update instructions when the bug in uglify is fixed?

I'll leave it to you 😃

[kzc]

I'm just surprised I haven't come across this earlier (or no one else has).

Likely because the uglify bug has only been present for a month: mishoo/UglifyJS#2560 (comment)

The fix is in the works and will be in the next release: mishoo/UglifyJS#2563

Uglify releases are generally made on the weekend.

[Conduitry]

Looks like this has been released in [email protected] (there's no [email protected] release yet though)

[Rich-Harris]

Excellent. @kzc —

Likely because the uglify bug has only been present for a month: mishoo/UglifyJS#2560 (comment)

I don't fully understand the distinction between the two bugs — are you saying that the only people affected by this are those who installed Uglify >= 3.1.6 and < 3.2.2? If so I think we're probably ok leaving Svelte unchanged and letting semver work its magic.

[kzc]

The bug I initially found was never reported in the wild and probably did not affect anyone in the 4 years it existed. Fixed by mishoo/UglifyJS#2562

The different bug discovered by @ekhaled affecting svelte existed for a month and was present beginning with 3.1.7. Fixed by mishoo/UglifyJS#2563

Both bugs will be fixed in 3.2.2.

Uglify doesn't follow semver.

[Rich-Harris]

Thanks. By 'letting semver work its magic', I mean that anyone with a dependency like "uglify-js": "^3.1.7" should get the fix without any action on their part, so we can probably close this without worrying about it too much.

[kzc]

@Rich-Harris off topic - any plans to revive Butternut? I see a lot of bug fix PRs piling up: https://github.com/Rich-Harris/butternut/pulls

Or are you just waiting for new maintainers to step up as has happened with Rollup and Bublé?

[Rich-Harris]

Maybe one day! The value of that project is less now that uglify-es is stable and widely-used — I'd still like to spend more time exploring the string-editing approach to minification as opposed to the AST-manipulation approach, as I think there potentially some decent performance wins, but Svelte is currently my main focus because a) it fills a gap in the ecosystem and b) as you say, other people have kindly taken over most maintenance of some of my other projects. At the moment I'm a happy uglify-es user.

I'll try and get round to that backlog of PRs soon though, thanks for the prod.

[kzc]

The world can never have enough minifiers. Competition is always good.

I think there's a limit to string-editing transformation when repeated complex transformation and inlining is involved. Eventually the original code string is transformed beyond usefulness and there's a combinatorial explosion of formatting fixes that are required as unrelated features react with each other. But every time I think something is impossible with that approach someone manages to get it working as you've successfully demonstrated with Rollup and Svelte. So what do I know.