Skip to content

Commit

Permalink
Fix Symbol compatibility detection under Terser
Browse files Browse the repository at this point in the history
The `String` constructor has a special case for `Symbol` conversions:
https://tc39.es/ecma262/#sec-string-constructor-string-value

    a. If NewTarget is undefined and value is a Symbol, return
       SymbolDescriptiveString(value).
    b. Let s be ? ToString(value).

Therefore, the only difference between `String(value)` and `value + ''`
is that the latter will throw for symbols.

Terser uses this to advantage by replacing invocations of `String(...)`
with `... + ''`. Terser, to its credit, has this behavior gated behind
the `unsafe` flag with the following documentation: "It enables some
transformations that might break code logic in certain contrived cases,
but should be fine for most code." For this particular code it is *not*
fine though.

Under WebPack, Terser is typically run per chunk rather than per file.
It is difficult or impossible to disable these optimizations for only
core-js, only for the entire bundle. This optimization causes the Symbol
constructor detection pathway to fail and fallback to string keys. After
everything is said and done we end up with a confusing error
message of "[object Generator] is not iterable".

The fix is simple, we simply bailout of this optimization in this
critical path.
  • Loading branch information
laverdet committed May 1, 2023
1 parent 3c5394d commit 9439c1d
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion packages/core-js/internals/symbol-constructor-detection.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
/* eslint-disable es/no-symbol -- required for testing */
var V8_VERSION = require('../internals/engine-v8-version');
var fails = require('../internals/fails');
var $String = String;

// eslint-disable-next-line es/no-object-getownpropertysymbols -- required for testing
module.exports = !!Object.getOwnPropertySymbols && !fails(function () {
var symbol = Symbol();
// Chrome 38 Symbol has incorrect toString conversion
// `get-own-property-symbols` polyfill symbols converted to object are not Symbol instances
return !String(symbol) || !(Object(symbol) instanceof Symbol) ||
// nb: Do not call `String` directly to avoid this being optimized out to `symbol+''` which will,
// of course, fail.
return !$String(symbol) || !(Object(symbol) instanceof Symbol) ||
// Chrome 38-40 symbols are not inherited from DOM collections prototypes to instances
!Symbol.sham && V8_VERSION && V8_VERSION < 41;
});

0 comments on commit 9439c1d

Please sign in to comment.