-
Notifications
You must be signed in to change notification settings - Fork 10k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[api-minor] Let Babel handle the necessary core-js
polyfills automatically
#16703
Conversation
…tically In the last couple of years we've been quicker to remove support for older browsers/environments, which means that at this point in time we don't bundle that many polyfills. (The polyfills are also generally simpler nowadays, ever since we removed support for e.g. Internet Explorer.) Rather than having to *manually* handle the polyfills, we can actually let Babel take care of bundling the necessary polyfills for us; please refer to https://babeljs.io/docs/babel-preset-env The only exception here is the Node.js-specific compatibility-code, which is moved into the `src/display/node_utils.js` file. This ought to be fine since workers are not available/used in Node.js-environments. *Please note:* For the `legacy`-builds this will increase the size of the *built* files, however that seems like a very small price to pay in order to simplify maintenance of the general PDF.js library.
With the changes in the previous patch the `isNodeJS`-helper no longer needs to live in its own file, which helps get rid of a closure in the *built* files.
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/e4de51ba35361a7/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/fd68460d7663338/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/fd68460d7663338/output.txt Total script time: 25.24 mins
Image differences available at: http://54.241.84.105:8877/fd68460d7663338/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/e4de51ba35361a7/output.txt Total script time: 38.94 mins
Image differences available at: http://54.193.163.58:8877/e4de51ba35361a7/reftest-analyzer.html#web=eq.log |
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/223d6ed58ee099a/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/223d6ed58ee099a/output.txt Total script time: 1.41 mins Published |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you.
In the last couple of years we've been quicker to remove support for older browsers/environments, which means that at this point in time we don't bundle that many polyfills. (The polyfills are also generally simpler nowadays, ever since we removed support for e.g. Internet Explorer.)
Rather than having to manually handle the polyfills, we can actually let Babel take care of bundling the necessary polyfills for us; please refer to https://babeljs.io/docs/babel-preset-env
The only exception here is the Node.js-specific compatibility-code, which is moved into the
src/display/node_utils.js
file. This ought to be fine since workers are not available/used in Node.js-environments.Please note: For the
legacy
-builds this will increase the size of the built files, however that seems like a very small price to pay in order to simplify maintenance of the general PDF.js library.