Skip to content

Commit

Permalink
Add tests, fix up fromMem to not need gross hack.
Browse files Browse the repository at this point in the history
  • Loading branch information
hildjj committed Jan 28, 2024
1 parent 231bed4 commit 56388d4
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 35 deletions.
57 changes: 24 additions & 33 deletions bin/fromMem.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
"use strict";

// Import or require module text from memory, rather than disk. Runs
// in a light sandbox that is likely easily escapable, but protects against
// slight oopsies like polluting the global namespace.
// in a node vm, very similar to how node loads modules.
//
// Ideas taken from the "module-from-string" and "eval" modules, neither of
// which were situated correctly to be used as-is.
Expand All @@ -12,8 +11,6 @@ const { Module } = require("module");
const path = require("path");
const url = require("url");

const IMPORTS = "___PEGGY___IMPORTS___";

// These already exist in a new, blank VM. Date, JSON, NaN, etc.
const vmGlobals = new vm
.Script("Object.getOwnPropertyNames(globalThis)")
Expand Down Expand Up @@ -63,11 +60,7 @@ globalContext.console = console;
function requireString(code, dirname, options) {
const m = new Module(options.filename, module); // Current module is parent.
// This is the function that will be called by `require()` in the parser.
m.require = (
// In node 12+, createRequire is documented.
// In node 10, createRequireFromPath is the least-undocumented approach.
Module.createRequire || Module.createRequireFromPath
)(options.filename);
m.require = Module.createRequire(options.filename);
const script = new vm.Script(code, { filename: options.filename });
return script.runInNewContext({
module: m,
Expand Down Expand Up @@ -108,7 +101,15 @@ async function importString(code, dirname, options) {
if (!vm.SourceTextModule) {
throw new Error("Start node with --experimental-vm-modules for this to work");
}
options.context[IMPORTS] = {};

const [maj, min] = process.version
.match(/v(\d+)\.(\d+)\.(\d+)/)
.slice(1)
.map(x => parseInt(x, 10));
if ((maj < 20) || ((maj === 20) && (min < 8))) {
throw new Error("Requires node.js 20.8+ or 21.");
}

const mod = new vm.SourceTextModule(code, {
identifier: options.filename,
context: vm.createContext(options.context),
Expand All @@ -123,24 +124,14 @@ async function importString(code, dirname, options) {
await mod.link(async(specifier, referencingModule) => {
const resolvedSpecifier = resolveIfNeeded(dirname, specifier);
const targetModule = await import(resolvedSpecifier);

// All of the below is to create a Module wrapper around the imported code
options.context[IMPORTS][specifier] = targetModule;

const stringifiedSpecifier = JSON.stringify(specifier);
const exportedNames = Object.keys(targetModule);
let targetModuleContent = "";
if (exportedNames.includes("default")) {
targetModuleContent += `export default ${IMPORTS}[${stringifiedSpecifier}].default;\n`;
}
const nonDefault = exportedNames.filter(n => n !== "default");
if (nonDefault.length > 0) {
targetModuleContent += `export const {${nonDefault.join(", ")}} = ${IMPORTS}[${stringifiedSpecifier}];`;
}

// @ts-expect-error: experimental
return new vm.SourceTextModule(targetModuleContent, {
identifier: resolvedSpecifier,
const exports = Object.keys(targetModule);

// DO NOT change function to () =>, or `this` will be wrong.
return new vm.SyntheticModule(exports, function() {
for (const e of exports) {
this.setExport(e, targetModule[e]);
}
}, {
context: referencingModule.context,
});
});
Expand All @@ -160,7 +151,7 @@ async function importString(code, dirname, options) {
module.exports = async function fromMem(code, options) {
options = {
format: "commonjs",
filename: __filename,
filename: `${__filename}-string`,
context: {},
includeGlobals: true,
globalExport: null,
Expand All @@ -173,6 +164,9 @@ module.exports = async function fromMem(code, options) {
...options.context,
};
}
options.context.global = options.context;
options.context.globalThis = options.context;

options.filename = path.resolve(options.filename);
const dirname = path.dirname(options.filename);

Expand All @@ -181,13 +175,10 @@ module.exports = async function fromMem(code, options) {
case "commonjs":
case "umd":
return requireString(code, dirname, options);
case "globals": {
const mod = requireString(code, dirname, options);
return mod[options.globalExport];
}
case "es":
// Returns promise
return importString(code, dirname, options);
// I don't care enough about amd and globals to figure out how to load them.
default:
throw new Error(`Unsupported output format: "${options.format}"`);
}
Expand Down
2 changes: 1 addition & 1 deletion bin/peggy.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env node --experimental-vm-modules --no-warnings
#!/usr/bin/env -S node --experimental-vm-modules --no-warnings

This comment has been minimized.

Copy link
@dasa

dasa Mar 26, 2024

Is -S needed here? I think this may be causing issues on Windows.

This comment has been minimized.

Copy link
@hildjj

hildjj Mar 26, 2024

Author Contributor

Yes, it's needed on Linux, which otherwise doesn't allow CLI parameters. Doesn't npm on Windows create a .bat file when you install? Can you open an issue and attach the created .bat file, please?

This comment has been minimized.

Copy link
@dasa

dasa Mar 26, 2024

I'm no longer able to repro. I think I was using an older version of npm missing this fix: npm/cmd-shim#54
Sorry for the noise.


"use strict";

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"lint": "eslint . --ext js,ts,mjs",
"ts": "tsc --build tsconfig.json",
"docs": "cd docs && npm run build",
"test": "jest",
"test": "node --experimental-vm-modules node_modules/jest/bin/jest.js",
"test:web": "cd web-test && npm test",
"test:all": "npm run test && npm run test:web",
"benchmark": "node ./benchmark/run_bench.js",
Expand Down
12 changes: 12 additions & 0 deletions test/cli/fixtures/imp.peggy
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{{
import opts from "./options.mjs";
// Cause importModuleDynamically to fire
const opts2 = await import("./options.mjs");
}}

foo='1' { return [
opts.cli_test.words,
opts2.default.cli_test.words,
// Needs to use import.meta to cause initializeImportMeta to fire.
import.meta.url.length > 0
]; }
9 changes: 9 additions & 0 deletions test/cli/fixtures/options.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default {
cli_test: {
words: ["zazzy"],
},
dependencies: {
j: "jest",
commander: "commander",
},
};
29 changes: 29 additions & 0 deletions test/cli/run.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,35 @@ Error: Expected "1" but end of input found.
});
});

it("handles tests that import other modules", async() => {
if ((await import("vm")).SourceTextModule) {
const grammar = path.join(__dirname, "fixtures", "imp.peggy");
try {
await exec({
args: ["--format", "es", "-t", "1", grammar],
expected: "[ [ 'zazzy' ], [ 'zazzy' ], true ]\n",
});
} catch (e) {
expect((e as Error).message).toMatch("Requires node.js 20.8+ or 21");
}
await exec({
args: ["--format", "amd", "-t", "1", grammar],
error: /Unsupported output format/,
});
await exec({
args: ["--format", "globals", "-t", "1", grammar],
error: /Unsupported output format/,
});
await exec({
args: ["--format", "bare", "-t", "1"],
stdin: "foo = '1'\n",
expected: "'1'\n",
});
} else {
throw new Error("Use --experimental-vm-modules");
}
});

it("handles grammar errors", async() => {
await exec({
stdin: "foo=unknownRule",
Expand Down

0 comments on commit 56388d4

Please sign in to comment.