Skip to content

Commit

Permalink
Allow super in eval() pre-serialization [fix]
Browse files Browse the repository at this point in the history
  • Loading branch information
overlookmotel committed Nov 13, 2023
1 parent 7a932d1 commit 740a138
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 10 deletions.
10 changes: 6 additions & 4 deletions lib/init/eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,12 @@ function evalDirect(args, filename, blockIdCounter, externalPrefixNum, evalDirec
} else if (varName === 'new.target') {
createNewTargetBinding(block);
allowNewTarget = true;
} else {
} else if (varName === 'super') {
// Don't create binding - `super` binding is created lazily
// TODO: Also need to set `superIsProto` and create a new temp var for `super` target
// (the external var could be shadowed inside `eval()` if prefix num is changing)
if (varName === 'super') state.currentSuperBlock = block;

state.currentSuperBlock = block;
} else {
// Whether var is function is not relevant because it will always be in external scope
// of the functions it's being recorded on, and value of `isFunction` only has any effect
// for internal vars
Expand Down Expand Up @@ -231,10 +232,11 @@ function compile(
// Parse code.
// If parsing fails, swallow error. Expression will be passed to `eval()`
// which should throw - this maintains native errors.
const allowSuper = !!state?.currentSuperBlock;
let ast;
try {
ast = parseImpl(
code, filename, false, false, allowNewTarget, false, isStrict, false, undefined
code, filename, false, false, allowNewTarget, allowSuper, false, isStrict, false, undefined
).ast;
} catch (err) {
return {code, shouldThrow: true, internalPrefixNum: externalPrefixNum, tempVars};
Expand Down
2 changes: 1 addition & 1 deletion lib/instrument/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function parse(code, options) {
assert(!sourceMaps || filename, 'options.filename must be provided when source maps enabled');

return parseImpl(
code, filename, isEsm, isCommonJs, isCommonJs, isJsx, isStrict, sourceMaps, inputSourceMap
code, filename, isEsm, isCommonJs, isCommonJs, false, isJsx, isStrict, sourceMaps, inputSourceMap
);
}

Expand Down
8 changes: 6 additions & 2 deletions lib/instrument/instrument.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const babelOptionsCache = {};
* @param {boolean} isCommonJs - `true` if is CommonJS
* @param {boolean} allowNewTarget - `true` if `new.target` can be used outside a function
* (CommmonJS or direct `eval()` within a function)
* @param {boolean} allowSuper - `true` if `super` can be used outside a function
* (direct `eval()` within a method)
* @param {boolean} isJsx - `true` if source contains JSX syntax
* @param {boolean} isStrict - `true` if strict mode
* @param {boolean} sourceMaps - `true` if source maps enabled
Expand All @@ -44,14 +46,16 @@ const babelOptionsCache = {};
* @throws {Error} - If parsing error
*/
function parseImpl(
code, filename, isEsm, isCommonJs, allowNewTarget, isJsx, isStrict, sourceMaps, inputSourceMap
code, filename, isEsm, isCommonJs, allowNewTarget, allowSuper, isJsx, isStrict,
sourceMaps, inputSourceMap
) {
// Parse code to AST
const ast = parse(code, {
sourceType: isEsm ? 'module' : 'script',
strictMode: isStrict,
allowReturnOutsideFunction: isCommonJs,
allowNewTargetOutsideFunction: allowNewTarget,
allowSuperOutsideMethod: allowSuper,
plugins: ['v8intrinsic', ...(isJsx ? ['jsx'] : [])]
});

Expand Down Expand Up @@ -97,7 +101,7 @@ function instrumentCodeImpl(
) {
// Parse code to AST
let {ast, sources} = parseImpl( // eslint-disable-line prefer-const
code, filename, isEsm, isCommonJs, isCommonJs, isJsx, isStrict, sourceMaps, inputSourceMap
code, filename, isEsm, isCommonJs, isCommonJs, false, isJsx, isStrict, sourceMaps, inputSourceMap
);

// Instrument AST
Expand Down
6 changes: 3 additions & 3 deletions lib/instrument/visitors/eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) {
const varDefsNodes = [];
for (const [varName, binding] of Object.entries(block.bindings)) {
if (varNamesUsed.has(varName)) continue;
if (isStrict && varName !== 'this' && isReservedWord(varName)) continue;
if (isStrict && varName !== 'this' && varName !== 'super' && isReservedWord(varName)) continue;

// Ignore `require` as it would render the function containing `eval()` un-serializable.
// Also ignore CommonJS wrapper function's `arguments` as that contains `require` too.
Expand All @@ -141,9 +141,9 @@ function instrumentEvalCall(callNode, block, fn, isStrict, canUseSuper, state) {
varDefsNodes.push(t.arrayExpression(varDefNodes));

// If var is external to function, record function as using this var.
// Ignore `new.target` as it's not possible to recreate.
// Ignore `new.target` and `super` as they're not possible to recreate.
// https://github.com/overlookmotel/livepack/issues/448
if (blockIsExternalToFunction && varName !== 'new.target') {
if (blockIsExternalToFunction && varName !== 'new.target' && varName !== 'super') {
activateBinding(binding, varName);
const externalVar = getOrCreateExternalVar(externalVars, block, varName, binding);
externalVar.isReadFrom = true;
Expand Down

0 comments on commit 740a138

Please sign in to comment.