Skip to content
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

upgrade AST<->ESTree translation #4870

Merged
merged 1 commit into from
Apr 25, 2021
Merged

upgrade AST<->ESTree translation #4870

merged 1 commit into from
Apr 25, 2021

Conversation

alexlamsl
Copy link
Collaborator

fixes #968

@alexlamsl alexlamsl force-pushed the moz branch 2 times, most recently from 43d5aba to a4e24e9 Compare April 25, 2021 14:38
@alexlamsl alexlamsl merged commit 324587f into mishoo:master Apr 25, 2021
@alexlamsl alexlamsl deleted the moz branch April 25, 2021 20:23
@kzc
Copy link
Contributor

kzc commented Apr 26, 2021

I attempted to vet this PR by running acorn on both sides of uglify-js minify while processing rollup-ts sources but it encountered errors:

--- a/test/release/rollup-ts.sh
+++ b/test/release/rollup-ts.sh
@@ -15,8 +15,12 @@ minify_in_situ() {
     for i in `find $DIRS -type f -name '*.ts' | grep -v '\.d\.ts'`
     do
         echo "$i"
-        node_modules/.bin/esbuild --loader=ts --target=node14 < "$i" \
-            | uglify-js $UGLIFY_OPTIONS -o "$i"
+        node_modules/.bin/esbuild --loader=ts --target=es2020 < "$i" \
+            | node_modules/.bin/acorn --ecma2020 --module \
+            | uglify-js -p spidermonkey $UGLIFY_OPTIONS -o spidermonkey \
+            | uglify-js -p spidermonkey \
+            | node_modules/.bin/acorn --ecma2020 --module \
+            | uglify-js -p spidermonkey -o "$i"
     done
 }

estree es2019 input works for this source file:

$ curl -s https://raw.githubusercontent.com/rollup/rollup/e9bedf202e4754caaa32d566c9eef0147e1ef3b9/cli/run/batchWarnings.ts | esbuild --loader=ts --target=es2019 | acorn --ecma2020 --module | uglify-js -p spidermonkey

but the equivalent estree es2020 input fails:

$ curl -s https://raw.githubusercontent.com/rollup/rollup/e9bedf202e4754caaa32d566c9eef0147e1ef3b9/cli/run/batchWarnings.ts | esbuild --loader=ts --target=es2020 | acorn --ecma2020 --module | uglify-js -p spidermonkey
ERROR: MOZ_TO_ME[node.type] is not a function

due to:

$ echo 'const id = warning.id || warning.loc?.file;' | node_modules/.bin/acorn --module --ecma2020 | uglify-js -p spidermonkey
ERROR: MOZ_TO_ME[node.type] is not a function

I can't make uglify-js accept the ESTree input for the following command regardless of esbuild target:

$ curl -s https://raw.githubusercontent.com/rollup/rollup/e9bedf202e4754caaa32d566c9eef0147e1ef3b9/cli/run/index.ts | esbuild --loader=ts --target=es2020 | acorn --ecma2020 --module | uglify-js -p spidermonkey
ERROR: MOZ_TO_ME[node.type] is not a function

due to:

$ echo 'async function a(){const {watch} = await import("./watch-cli");}' | node_modules/.bin/acorn --module --ecma2020 | uglify-js -p spidermonkey
ERROR: MOZ_TO_ME[node.type] is not a function

Feel free to takeover the patch above to further test and get it working. The number of steps in the verification pipeline may seem excessive but it's the only way I'm aware of to filter both the spidermonkey input and spidermonkey output for the same minify process through acorn. It also has the advantage of exercising uglify-js to parse its own spidermonkey output. Note that esbuild --target=es2020 is preferable to --target=node14 because of NodeJS-specific object spread/rest transforms in esbuild.

Edit: corrected the rollup-ts patch to use --target=es2020 instead of --target=es2019.

@kzc
Copy link
Contributor

kzc commented Apr 26, 2021

The following patch improves the usability of unknown ESTree node type errors:

--- a/lib/mozilla-ast.js
+++ b/lib/mozilla-ast.js
@@ -1160,7 +1160,14 @@
 
-    function from_moz(node) {
-        FROM_MOZ_STACK.push(node);
-        var ret = node != null ? MOZ_TO_ME[node.type](node) : null;
+    function from_moz(moz) {
+        FROM_MOZ_STACK.push(moz);
+        var native = null;
+        if (moz) {
+            if (HOP(MOZ_TO_ME, moz.type)) {
+                native = MOZ_TO_ME[moz.type](moz);
+            } else {
+                js_error("Unknown mozilla node type '" + moz.type + "' at position " + moz.start);
+            }
+        }
         FROM_MOZ_STACK.pop();
-        return ret;
+        return native;
     }
$ echo 'const id = warning.id || warning.loc?.file;' | node_modules/.bin/acorn --module --ecma2020 | uglify-js -p spidermonkey -p spidermonkey
ERROR: Unknown mozilla node type 'ChainExpression' at position 25
$ echo 'async function a(){const {watch} = await import("./watch-cli");}' | node_modules/.bin/acorn --module --ecma2020 | uglify-js -p spidermonkey
ERROR: Unknown mozilla node type 'ImportExpression' at position 41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ES6] UglifyJS.AST_Node.from_mozilla_ast(acorn ast) compatibility issues
2 participants