closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

Object prototype methods in compiled output fail due to undefined arg (-O ADVANCED)

Open ctjlewis opened this issue 3 years ago • 1 comments

Issue

I have rolled an ES module into one large file, including thousands of NPM deps (very large AST), and I am feeding it to the compiler. The output is not executable due to calls like Object.keys(undefined) existing in the output.

Here is the original source (intentional type mismatches as a test):

import marked from 'marked';
import sanitizeHtml from 'sanitize-html';

/**
 * A Markdown note.
 *
 * @param {string} text
 * The text to display.
 *
 * @return {string}
 */
function NoteWithMarkdown(text) {
  const html = sanitizeHtml(
      marked(text),
  );
  return html;
}

/** @type {string} */
const a = 5;
console.log(/** @type {number} */ (a));

console.log(
    NoteWithMarkdown('#h1\n##h2'),
);

Precompiled input

The rolled-up dev output fed to the compiler is here: https://pastebin.com/raw/sZRgUjTd

It executes as expected, logging data to the console. This can be compiled with -O ADVANCED, but the output fails at execution time (via node dist/exe.js) due to the issue mentioned above. With --debug --formatting PRETTY_PRINT flags:

file:///home/christian/PersonalProjects/markdown-test/dist/exe.js:3025
    var $keys$$ = Object.keys($map$jscomp$6$$).join("|"), $replace$jscomp$1$$ = $getReplacer$$($map$jscomp$6$$), $re$$ = new RegExp("&(?:" + ($keys$$ + "|#[xX][\\da-fA-F]+|#\\d+);"), "g");
                         ^

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at $getStrictDecoder$$ (file:///home/christian/PersonalProjects/markdown-test/dist/exe.js:3025:26)
    at file:///home/christian/PersonalProjects/markdown-test/dist/exe.js:3045:37
    at $createCommonjsModule$$ (file:///home/christian/PersonalProjects/markdown-test/dist/exe.js:926:7)
    at file:///home/christian/PersonalProjects/markdown-test/dist/exe.js:3020:17
    at ModuleJob.run (internal/modules/esm/module_job.js:146:23)
    at async Loader.import (internal/modules/esm/loader.js:165:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5)

Compiled output

The source for this file can be found here (too big for PasteBin): https://drive.google.com/file/d/16NLCW824NQDPv7yYMP1jL6tX7FMbKKQS/view?usp=sharing

This error persists through multiple calls to Object prototype methods including setPrototypeOf etc. After naively patching the AST to use calls like Object.keys(val || {}), we get to an error regarding the library import:

file:///home/christian/PersonalProjects/markdown-test/dist/exe.patched.js:11797
  var $parser$jscomp$4$$ = new $lib$5$$.$Parser$({
                           ^

TypeError: $lib$5$$.$Parser$ is not a constructor
    at sanitizeHtml (file:///home/christian/PersonalProjects/markdown-test/dist/exe.patched.js:11797:28)
    at file:///home/christian/PersonalProjects/markdown-test/dist/exe.patched.js:12002:3
    at ModuleJob.run (internal/modules/esm/module_job.js:146:23)
    at async Loader.import (internal/modules/esm/loader.js:165:24)
    at async Object.loadESM (internal/process/esm_loader.js:68:5)

And this error I have no idea how to solve, it is quite beyond me.

Like the other issue I opened today, I understand this is low priority, but I want to document it anyway. This is effectively stress-testing due to the amount of real-life code that is being fed into the compiler in this example, and it is unsurprising that one or two errors bubble up and break the output. However, once the compiler can produce acceptable output for this example, it would likely be a while before a similar bug is discovered - it is extremely rare that the compiler produces outputs with execution-time errors that do not exist in the source in my experience, and only occurs for extremely advanced (and technically unsupported) use cases like this, where we are piping in thousands of real-life NPM dependencies.

ctjlewis avatar Dec 27 '20 22:12 ctjlewis

My guess is that this is related to property renaming + whatever system you're using to transpile modules.

From the input:

exports.Parser=void 0;Object.defineProperty(exports,"Parser",{enumerable:true,get:function(){return Parser_1.Parser;}});

From the output:

$exports$jscomp$23$$.$Parser$ = void 0;
  Object.defineProperty($exports$jscomp$23$$, "Parser", {enumerable:!0, get:function() {
    return $Parser_1$$.$Parser$;
  }});

Note the $Parser$ vs. "Parser" in the output.

Closure Compiler supports renaming properties defined via Object.defineProperties, but never renames the string literal passed into Object.defineProperty.

One possible solution is wrapping "Parser" and other such quoted strings in goog.reflect.objectProperty('Parser', exports)

lauraharker avatar Dec 29 '20 18:12 lauraharker