Question: Why concatenate Symbols with a string without assigning it to a variable?

[lukastaegert]

When making sure that Rollup is able to handle this shim without unexpected tree-shaking, I became upon this line, which is removed by Rollup:

es5-shim/es5-shim.js

Line 2027 in d5297ad

'' + str; // jscs:ignore disallowImplicitTypeConversion

Is this line actually correct or should it rather be something like str = '' + str?

Otherwise I fail to see what the side-effect should be. If it is a thrown error, I am missing the try-statement. Please tell me if I overlook anything here.

[ljharb]

It's correct, because implicit String coercion causes Symbols to throw, and this line is intended to replicate that exception. Removing this line will cause code that shouldn't work, to start working.

[lukastaegert]

Thanks for explaining. Not sure if we are going to fix this one soon, everything else here is working fine, though. A simple way to prevent tree-shaking could be to put this in a place where the return value is used, e.g.

function parseInt(str, radix) {
  var string = trim(String(typeof str === 'symbol' ? '' + str : str));
  var defaultedRadix = $Number(radix) || (hexRegex.test(string) ? 16 : 10);
  return origParseInt(string, defaultedRadix);
};

I do not suppose this is something you would consider? Tackling this via static analysis from our side will be quite hard, and deoptimizing all string concatenations is a really high price for a runtime error that should not occur in proper code.

[ljharb]

How often does a noop string concat occur in code such that you need to ever remove it anywhere? Is that really a common pattern?

[lukastaegert]

Probably as often as this runtime error—usually due to an error on the developer's side 😜
But yes, we could check for this specific pattern.

[ljharb]

I think it's safe to leave that to a linter catching noop statements :-)

[lukastaegert]

You would be surprised as to how many people do not lint...

Still, catching it now on our side with a specific heuristic: rollup/rollup#2917