Attempt to fix the Array.prototype.includes and String.prototype.includes polyfills (issue 9514, 9516)
#9520
Conversation
| @@ -131,7 +131,7 @@ PDFJS.compatibilityChecked = true; | |||
| if (String.prototype.includes) { | |||
| return; | |||
| } | |||
| String.prototype.includes = require('core-js/fn/string/includes'); | |||
There was a problem hiding this comment.
Just remove the String.prototype.includes = part and this should work.
When you import core-js/fn/string/includes, then String.prototype.includes is already overwritten, which can be seen using the following example:
$ node -e 'delete String.prototype.includes; console.log("".includes); require("core-js/fn/string/includes"); console.log("abc".includes("a"))'
undefined
true
The exported function from require('core-js/fn/string/includes') does not use this, but expects the first parameter to be the string, and the other parameters to be the parameters for the string method. I.e. require('core-js/fn/string/includes')("abc", "b");. With the current (wrong) code, it becomes require('core-js/fn/string/includes').call("abc", "b"), which becomes (since thisis ignored)require('core-js/fn/string/includes')("b"), i.e. equivalent to "b".includes(undefined)`.
The documentation at https://github.com/zloirock/core-js#usage also shows that the intended usage for the default mode is to just call require and not use its return value.
There was a problem hiding this comment.
Thank you; I've made the suggested changes and everything seems to work just fine now!
…ncludes` polyfills (issue 9514, 9516) I don't understand why the previous way importing the polyfills didn't work, and I don't have time to try and figure it out, however this patch seems to fix things. Fixes 9514. Fixes 9516.
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/4ed073dd5007fac/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/4ed073dd5007fac/output.txt Total script time: 2.59 mins Published |
|
Squashed two bugs in one PR, good one! |
…ills Attempt to fix the `Array.prototype.includes` and `String.prototype.includes` polyfills (issue 9514, 9516)
I don't understand why the previous way importing the polyfills didn't work, and I don't have time to try and figure it out, however this patch seems to fix things.
Fixes #9514.
Fixes #9516.