Static class properties not referenced properly in obfuscated code

[tzvetelin-vassilev]

Bug report or Feature request? Bug report
Version 5.16.8

The problem could be observed with a lot of code. Small piece cannot reproduce the problem. I'll try to explain with some pseudo code and short description of it.

The problem affects only es syntax. Example class A constructor has an argument 'type' and static property 'Type'. After bundling something curious is observed. import 'ns' is transformed as 'e' in obfuscated code. For some reason class A is also named 'e'. Terser tries to fix this with 'let Ht = class e', so class 'e' is 'Ht'. Next references looks fine and on class 'Ht' is assigned property 'Type'. But in constructor we can find 'e.Type.TypeA', which should be 'Ht.Type.TypeA'. The questions here is if namespace 'e' is reserved already, why is needed this assignment: 'let Ht = class e', it could be just 'class Ht'. If keeping of the assignment is needed for some reason, static props should be properly referenced from the transforming code.

terser input

// ========= X.mjs begin

// some imports ...
import * as ns from "some-library"

class X {
  // X stuff
}

export default X

// ========= X.mjs end

// ========= A.mjs begin

class A {
  constructor(type) {
    if (type == A.Type.TypeA) {
      // init TypeA case
    }
    else {
      // init TypeB case
    }
  }
}

A.Type = {
  TypeA: 1,
  TypeB: 2
};

export default A

// ========= A.mjs end

terser output

// some imports ...
import * as e from "some-library";

class X {
	// X stuff
}

// ... a lot of code

let Ht = class e {
  constructor(Vt) {
    if (Vt == e.Type.TypeA) {
      // init TypeA case
    }
    else {
      // init TypeB case
    }
  }
}

Ht.Type = {
  TypeA: 1,
  TypeB: 2
};

Outside of any other context this is properly interpreted from browser, but if this code is imported in wider context then 'e.Type.TypeA' is ambiguous and could be treated as 'ns.Type.TypeA' which results to exception.

Expected result
From my point of view the best result would be

let Ht = class e {...}

to be

class Ht {...}

[dasa]

Are you also using Rollup, and then feeding the output to Webpack? I ran into this issue here: webpack/webpack#16763 A potential fix could be for Terser to not shadow the imports.

[tzvetelin-vassilev]

Yes, the same issue. This potential fix is welcome.

[fabiosantoscode]

This seems safe to me. The name e can be shadowed and reused inside the class, because classes make their name available within, much like `(function fName() { /* use fName here */ }).

So He and the class's name e will always have the same value in this case.

If you start referencing ns inside that class you'll see that Terser will not do any shadowing.

[fabiosantoscode]

As a side note, Terser doesn't create let variables. In some circumstances it might create var. I've added the suboptimal output label because I think something in your toolchain is adding it and it could be removed by Terser.

[dasa]

I believe this change started in Rollup at rollup/rollup#4827 and that this isn't a Terser bug, but is a Webpack bug, and any change to Terser, would be a workaround to avoid the Webpack bug, so I'd completely understand if you don't want to make any changes.

[tzvetelin-vassilev]

Just to confirm, rollup creates let variable, this doesn't comes from terser side.