overlookmotel / livepack

Serialize live running code to Javascript
MIT License
42 stars 1 forks source link

Instrumented CommonJS code fails to run if top-level function called `require` #567

Closed overlookmotel closed 9 months ago

overlookmotel commented 9 months ago

Weird but legal CommonJS input:

module.exports = {x: 1};
function require() {}

When instrumented code is run, this throws ReferenceError: Cannot access 'livepack_tracker' before initialization.

Instumented code:

const [livepack_tracker, livepack_getScopeId] = require("/path/to/livepack/lib/init/index.js")("/path/to/src/index.js", require, 5, 0);
module.exports = {x: 1};
function/*livepack_track:3;0;/path/to/src/index.js*/require() {
  livepack_tracker(livepack_getFnInfo_3, () => []);
}

require("/path/to/livepack/lib/init/index.js") calls the locally-defined require function instead of CommonJS require.

Fix would be if instrumentation renamed the local require function, and then assigned it to a var:

const [livepack_tracker, livepack_getScopeId] = require("/path/to/livepack/lib/init/index.js")("/path/to/src/index.js", require, 5, 0);
var require = Object.defineProperty(livepack_temp_1, 'name', {value: 'require'});

module.exports = {x: 1};
function/*livepack_track:3;0;/path/to/src/index.js*/livepack_temp_1() {
  livepack_tracker(livepack_getFnInfo_3, () => []);
}
overlookmotel commented 9 months ago

To avoid a problem if Object is a local var too (!), add a method to livepack_getScopeId for this purpose and call it rather than using Object.defineProperty directly in instrumented code:

livepack_getScopeId.nameRequire = require => {
  return Object.defineProperty(require, 'name', {value: 'require'});
};
overlookmotel commented 9 months ago

Should only rename function declarations at top level, and not inside nested blocks.

A function declaration in a nested block will not be hoisted up, because it's blocked from doing that by the parameter called require in the CommonJS wrapper function.

{
  function require() {}
  require.x = 1;
}
assert(require.x === undefined);
overlookmotel commented 9 months ago

Implemented on require-funcs branch. Just needs tests.