Temp variable re-usage breaks JS when building into single file

[sosnovsky]

🔎 Search Terms

static

🕗 Version & Regression Information

• This changed between versions 5.2.0-beta and 5.2.1-rc. I found that this issue appeared in 5.2.0-dev.20230805 build, probably caused by merging Fix class name references #55262

⏯ Playground Link

https://www.typescriptlang.org/play?#code/PTAEGMEMBdwCwHTQM4ChwBtLOaAwjPKAN6qqgUAOArgEYYCW4oy0MToATgKaUD2naAFFOnUAF5QACgCUALlC0+fDN0gA7CQD4S5Cl27RqnTQViI+6kZwEBJddG4nIGABIaAJqs6yA3HoBffz0aeg5WdmZLazsHJ3UXd3UvJwlpeUVlVQ1tXX0DIxNQaE5qbn8KILIqOkZmCOgODz5uZAAlbgArbnBoNOwAT3VmWVzSfJ5CzQAzF2RuQP8AshBQDT5oOCcAWihzJDRMbFwAQXUNrc4zInGasPq2RuYefkEAZT4AW0M4BnUAczSsgUShUak04h0twok2MmjOFyc1ws6g+302f3+SRSPhkFVAVRCtXCjw4ljRP0x2O8QIyoOyEKhehhhjhxVK5UW1VAoTqLFJzGarQ63V6-WQQxGMjGzIKbNmGHmXOWqFW4C+lAYqmKW3moAATKBplrWqA-tA+GsWJjtZZuKAAO4MTagABEKGYe3gBzW502Oy9iBQoG22z41GgADETcVWtAEJ1cKHPnwPNRted1PbQ2xOP9DKBWvqAAwARgA7K6gA

💻 Code

// catch.ts
class Catch {

   public static reportErr = (): boolean => {
    return Catch.onErrorInternalHandler();
  };

  public static onErrorInternalHandler = (): boolean => {
    return true;
  };

  public static doesReject = async () => {
    return false
  };
}

// another-catch.ts
class AnotherCatch {
   public static reportSomething = (): boolean => {
    return AnotherCatch.onSomethingHandler();
  };

  public static onSomethingHandler = (): boolean => {
    return true;
  };

  public static doesReject = async () => {
    return false
  };
}

// compile these 2 files into a single one with "tsc catch.ts another-catch.ts --outFile test.js --module none --target es2017"

🙁 Actual behavior

In compiled JS it converts to:

var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return _a.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};
Catch.doesReject = async () => {
    return false;
};
var _a;
class AnotherCatch {
}
_a = AnotherCatch;
AnotherCatch.reportSomething = () => {
    return _a.onSomethingHandler();
};
AnotherCatch.onSomethingHandler = () => {
    return true;
};
AnotherCatch.doesReject = async () => {
    return false;
};

Which leads to _a.onErrorInternalHandler is not a function error when trying to perform Catch.reportErr() later in the code.

🙂 Expected behavior

In 5.2.0-beta it compiled to

var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return Catch.onErrorInternalHandler();
};

And it worked well.

Additional information about the issue

No response

[andrewbranch]

Could you provide a TypeScript playground link that demonstrates the issue?

[sosnovsky]

Could you provide a TypeScript playground link that demonstrates the issue?

Yeah, I was able to reproduce this issue with a minimal amount of code - playground

TS Code:

class Catch {
   public static reportErr = (): boolean => {
    return Catch.onErrorInternalHandler();
  };

  public static onErrorInternalHandler = (): boolean => {
    return true;
  };

  public static doesReject = async () => {
    return false
  };
}

Compiles in 5.2.2 to such JS:

"use strict";
var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return _a.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};
Catch.doesReject = async () => {
    return false;
};

It causes error _a.onErrorInternalHandler is not a function.
In 5.1.6 it compiles to

"use strict";
var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return Catch.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};
Catch.doesReject = async () => {
    return false;
};

And it works well.

But if I remove doesReject method, then it compiles well in 5.2.2 too:

class Catch {
   public static reportErr = (): boolean => {
    return Catch.onErrorInternalHandler();
  };

  public static onErrorInternalHandler = (): boolean => {
    return true;
  };
}

Compiled JS:

"use strict";
class Catch {
}
Catch.reportErr = () => {
    return Catch.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};

[MartinJohns]

It causes error _a.onErrorInternalHandler is not a function.

I can't reproduce this. Neither in 5.2.2 nor in the nightly version. Your playground link does not show this error either.

[sosnovsky]

Ah, it happens because in our project we compile TS files to separate JS files and then merge these JS files into a single one. As a result, _a variable is reassigned and causes _a.onErrorInternalHandler is not a function error.

For example:

catch.ts

class Catch {
   public static reportErr = (): boolean => {
    return Catch.onErrorInternalHandler();
  };

  public static onErrorInternalHandler = (): boolean => {
    return true;
  };

  public static doesReject = async () => {
    return false
  };
}

another-catch.ts

class AnotherCatch {
   public static reportSomething = (): boolean => {
    return AnotherCatch.onSomethingHandler();
  };

  public static onSomethingHandler = (): boolean => {
    return true;
  };

  public static doesReject = async () => {
    return false
  };
}

Compiled files:

catch.js

var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return _a.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};
Catch.doesReject = async () => {
    return false;
};

another-catch.js

var _a;
class AnotherCatch {
}
_a = AnotherCatch;
AnotherCatch.reportSomething = () => {
    return _a.onSomethingHandler();
};
AnotherCatch.onSomethingHandler = () => {
    return true;
};
AnotherCatch.doesReject = async () => {
    return false;
};

After merging these files we have such JS:

/* catch.js */
var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return _a.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};
Catch.doesReject = async () => {
    return false;
};

/* another-catch.js */
var _a;
class AnotherCatch {
}
_a = AnotherCatch;
AnotherCatch.reportSomething = () => {
    return _a.onSomethingHandler();
};
AnotherCatch.onSomethingHandler = () => {
    return true;
};
AnotherCatch.doesReject = async () => {
    return false;
};

And when later I try to perform Catch.reportErr() it fails, because _a was reassigned to AnotherCatch.

As I understand, this change was done for some optimisations, but it breaks workflow when compiled JS files are later merged into single one. Is it possible to disable this behavior for such cases, maybe with some configuration option?

[andrewbranch]

What are you using to do this merging? This seems like something a dedicated tool should understand and catch. The only way you can change the emit is by targeting a new ES version where static fields are natively supported.

[MartinJohns]

This is definitely a bug, or should be treated differently. Using outFile to bundle the output the resulting file will also re-use the _a identifier, so the issue also appears without any additional tools.

Creating the two files as mentioned and compiling it with tsc catch.ts another-catch.ts --outFile test.js --module none --target es2017 results in this output:

var _a;
class Catch {
}
_a = Catch;
Catch.reportErr = () => {
    return _a.onErrorInternalHandler();
};
Catch.onErrorInternalHandler = () => {
    return true;
};
Catch.doesReject = async () => {
    return false;
};
var _a;
class AnotherCatch {
}
_a = AnotherCatch;
AnotherCatch.reportSomething = () => {
    return _a.onSomethingHandler();
};
AnotherCatch.onSomethingHandler = () => {
    return true;
};
AnotherCatch.doesReject = async () => {
    return false;
};

[andrewbranch]

Also, depending on your target, you could use static methods instead of arrow functions assigned to static fields, as static methods are supported in an older ES version than static fields.

[sosnovsky]

This is definitely a bug, or should be treated differently. Using outFile to bundle the output the resulting file will also re-use the _a identifier, so the issue also appears without any additional tools.

Creating the two files as mentioned and compiling it with tsc catch.ts another-catch.ts --outFile test.js --module none --target es2017 results in this output

Yeah, I was able to reproduce it with such setup, so it breaks compiling to a single JS file too

[andrewbranch]

I guess it would break as well by emitting two individual files and including them one after another in two <script> tags. @rbuckton or @weswigham, what do we usually do about temp variable clashes like this?

[sosnovsky]

@andrewbranch are there any updates on this issue?