Skip to content

Commit

Permalink
Replace the webpack+acorn transform with a Babel plugin
Browse files Browse the repository at this point in the history
This commit converts the pdfjsdev-loader transform into a Babel plugin,
to skip a AST->string->AST round-trip.

Before this commit, the webpack build process was:
1. Babel parses the code
2. Babel transforms the AST
3. Babel generates the code
4. Acorn parses the code
5. pdfjsdev-loader transforms the AST
6. @javascript-obfuscator/escodegen generates the code
7. Webpack parses the file
8. Webpack concatenates the files

After this commit, it is reduced to:
1. Babel parses the code
2. Babel transforms the AST
3. babel-plugin-pdfjs-preprocessor transforms the AST
4. Babel generates the code
5. Webpack parses the file
6. Webpack concatenates the files

This change improves the build time by ~25% (tested on MacBook Air M2):
- `gulp lib`: 3.4s to 2.6s
- `gulp dist`: 36s to 29s
- `gulp generic`: 5.5s to 4.0s
- `gulp mozcentral`: 4.7s to 3.2s

The new Babel plugin doesn't support the `saveComments` option of
pdfjsdev-loader, and it just always discards comments. Even though
pdfjsdev-loader supported multiple values for that option, it was
effectively ignored due to `acorn` dropping comments by default.
  • Loading branch information
nicolo-ribaudo committed Jan 23, 2024
1 parent f5bb9bc commit f724ae9
Show file tree
Hide file tree
Showing 16 changed files with 282 additions and 474 deletions.
454 changes: 174 additions & 280 deletions external/builder/babel-plugin-pdfjs-preprocessor.mjs

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions external/builder/fixtures_esprima/blocks-expected.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
function test() {
"test";
"1";
"2";
"3";
if ("test") {
"5";
}
"4";
"test";
"1";
"2";
"3";
if ("test") {
"5";
}
"4";
}
22 changes: 11 additions & 11 deletions external/builder/fixtures_esprima/comments-expected.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
function f1() {
"1";
"2";
"1";
"2";
}
function f2() {
"1";
"2";
"1";
"2";
}
function f3() {
if ("1") {
"1";
}
"2";
if ("3") {
"4";
}
if ("1") {
"1";
}
"2";
if ("3") {
"4";
}
}
2 changes: 1 addition & 1 deletion external/builder/fixtures_esprima/constants-expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ var i = true;
var j = false;
var k = false;
var l = true;
var m = '1' === true;
var m = false;
var n = false;
var o = true;
22 changes: 15 additions & 7 deletions external/builder/fixtures_esprima/deadcode-expected.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
function f1() {
}
function f1() {}
function f2() {
return 1;
return 1;
}
function f3() {
var i = 0;
throw "test";
var i = 0;
throw "test";
}
function f4() {
var i = 0;
var i = 0;
}
var obj = {
method1() {},
method2() {}
};
class C {
method1() {}
method2() {}
}

var arrow1 = () => {};
var arrow2 = () => {};
12 changes: 12 additions & 0 deletions external/builder/fixtures_esprima/deadcode.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,15 @@ function f4() {
var j = 0;
}

var obj = {
method1() { return; var i = 0; },
method2() { return; },
};

class C {
method1() { return; var i = 0; }
method2() { return; }
}

var arrow1 = () => { return; var i = 0; };
var arrow2 = () => { return; };
16 changes: 11 additions & 5 deletions external/builder/fixtures_esprima/evals-expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@ var b = true;
var c = true;
var d = false;
var e = true;
var f = 'text';
var f = "text";
var g = {
"obj": { "i": 1 },
"j": 2
obj: {
i: 1
},
j: 2
};
var h = {
test: "test"
};
var h = { 'test': 'test' };
var i = '0';
var j = { "i": 1 };
var j = {
i: 1
};
2 changes: 1 addition & 1 deletion external/builder/fixtures_esprima/evals.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{ 'test': 'test' }
{ "test": "test" }
10 changes: 5 additions & 5 deletions external/builder/fixtures_esprima/ifs-expected.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
if ('test') {
"1";
"1";
}
{
"1";
"1";
}
{
"1";
"1";
}
;
{
"2";
"2";
}
;
if ('1') {
"1";
"1";
}
8 changes: 3 additions & 5 deletions external/builder/fixtures_esprima/importalias-expected.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { Test } from 'import-name';
import { Test } from "import-name";
import { Test2 } from './non-alias';
export {
Test3
} from 'import-name';
var Imp = require('import-name');
export { Test3 } from "import-name";
var Imp = require("import-name");
var Imp2 = require('./non-alias');
6 changes: 6 additions & 0 deletions external/builder/test-fixtures.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ files.forEach(function (expectationFilename) {
if (out !== expectation) {
errors++;

// Allow regenerating the expected output using
// OVERWRITE=true node ./external/builder/test-fixtures.mjs
if (process.env.OVERWRITE) {
fs.writeFileSync(expectationFilename, out + "\n");
}

console.log("Assertion failed for " + inFilename);
console.log("--------------------------------------------------");
console.log("EXPECTED:");
Expand Down
8 changes: 7 additions & 1 deletion external/builder/test-fixtures_esprima.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { fileURLToPath } from "url";
import fs from "fs";
import path from "path";
import { preprocessPDFJSCode } from "./preprocessor2.mjs";
import { preprocessPDFJSCode } from "./babel-plugin-pdfjs-preprocessor.mjs";

const __dirname = path.dirname(fileURLToPath(import.meta.url));

Expand Down Expand Up @@ -48,6 +48,12 @@ files.forEach(function (expectationFilename) {
if (out !== expectation) {
errors++;

// Allow regenerating the expected output using
// OVERWRITE=true node ./external/builder/test-fixtures_esprima.mjs
if (process.env.OVERWRITE) {
fs.writeFileSync(expectationFilename, out + "\n");
}

console.log("Assertion failed for " + inFilename);
console.log("--------------------------------------------------");
console.log("EXPECTED:");
Expand Down
41 changes: 0 additions & 41 deletions external/webpack/pdfjsdev-loader.mjs

This file was deleted.

40 changes: 22 additions & 18 deletions gulpfile.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
*/
/* eslint-env node */

import {
babelPluginPDFJSPreprocessor,
preprocessPDFJSCode,
} from "./external/builder/babel-plugin-pdfjs-preprocessor.mjs";
import { exec, spawn, spawnSync } from "child_process";
import autoprefixer from "autoprefixer";
import babel from "@babel/core";
Expand All @@ -30,7 +34,6 @@ import postcssDirPseudoClass from "postcss-dir-pseudo-class";
import postcssDiscardComments from "postcss-discard-comments";
import postcssNesting from "postcss-nesting";
import { preprocess } from "./external/builder/builder.mjs";
import { preprocessPDFJSCode } from "./external/builder/preprocessor2.mjs";
import rename from "gulp-rename";
import replace from "gulp-replace";
import rimraf from "rimraf";
Expand Down Expand Up @@ -209,10 +212,11 @@ function createWebpackConfig(
const isModule = output.library?.type === "module";
const skipBabel = bundleDefines.SKIP_BABEL;

// `core-js`, see https://github.com/zloirock/core-js/issues/514,
// should be excluded from processing.
const babelExcludes = ["node_modules[\\\\\\/]core-js"];
const babelExcludeRegExp = new RegExp(`(${babelExcludes.join("|")})`);
const babelExcludeRegExp = [
// `core-js`, see https://github.com/zloirock/core-js/issues/514,
// should be excluded from processing.
/node_modules[\\/]core-js/,
];

const babelPresets = skipBabel
? undefined
Expand All @@ -227,7 +231,15 @@ function createWebpackConfig(
},
],
];
const babelPlugins = [];
const babelPlugins = [
[
babelPluginPDFJSPreprocessor,
{
rootPath: __dirname,
defines: bundleDefines,
},
],
];

const plugins = [];
if (!disableLicenseHeader) {
Expand Down Expand Up @@ -335,14 +347,6 @@ function createWebpackConfig(
targets: BABEL_TARGETS,
},
},
{
loader: path.join(__dirname, "external/webpack/pdfjsdev-loader.mjs"),
options: {
rootPath: __dirname,
saveComments: false,
defines: bundleDefines,
},
},
],
},
// Avoid shadowing actual Node.js variables with polyfills, by disabling
Expand Down Expand Up @@ -463,7 +467,6 @@ function createSandboxExternal(defines) {
const licenseHeader = fs.readFileSync("./src/license_header.js").toString();

const ctx = {
saveComments: false,
defines,
};
return gulp
Expand Down Expand Up @@ -1572,13 +1575,15 @@ function buildLibHelper(bundleDefines, inputStream, outputDir) {
}
function preprocessLib(content) {
const skipBabel = bundleDefines.SKIP_BABEL;
content = preprocessPDFJSCode(ctx, content);
content = babel.transform(content, {
sourceType: "module",
presets: skipBabel
? undefined
: [["@babel/preset-env", { loose: false, modules: false }]],
plugins: [babelPluginReplaceNonWebpackImport],
plugins: [
babelPluginReplaceNonWebpackImport,
[babelPluginPDFJSPreprocessor, ctx],
],
targets: BABEL_TARGETS,
}).code;
content = content.replaceAll(
Expand All @@ -1589,7 +1594,6 @@ function buildLibHelper(bundleDefines, inputStream, outputDir) {
}
const ctx = {
rootPath: __dirname,
saveComments: false,
defines: bundleDefines,
map: {
"pdfjs-lib": "../pdf.js",
Expand Down
Loading

0 comments on commit f724ae9

Please sign in to comment.