Remove Symbol polyfill in module namespaces #3135
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3135 +/- ##
==========================================
- Coverage 89.32% 89.31% -0.01%
==========================================
Files 165 165
Lines 5731 5726 -5
Branches 1740 1737 -3
==========================================
- Hits 5119 5114 -5
Misses 379 379
Partials 233 233
Continue to review full report at Codecov.
|
| }${_}}${_}});${n}`; | ||
| output += `${callee}(${name});`; | ||
| } | ||
| let output = `${options.varOrConst} ${name} = ${`${callee}({${n}${members.join( |
There was a problem hiding this comment.
For the compact output, there are still a few empty spaces that can be replaced with ${_}, which will be replaced with an empty string for compact: true output (around the =). Also, is the template string inside the template string intended?
There was a problem hiding this comment.
But I see the ${_} were already missing in the original form.
There was a problem hiding this comment.
Yeah, nested template string is intended and were there already. You are right, ${_} were missing but should I update?
There was a problem hiding this comment.
Also if getting rid of the nested template string is possible, that would be nice. Looks like this to me.
mkubilayk
force-pushed
the
tostring-polyfill
branch
from
b0e9179
to
dc67099
Compare
|
@lukastaegert Thanks for the review. Incorporated your feedback. |
|
Nice feature. Documentation missing tho! :) |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Fixes #3097.
Description
This simplifies generated code for module namespaces by removing the
Symbolpolyfill. It is technically a breaking change but it was agreed in #3097 (comment).