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

fix(codegen): better error messages for internals using module.exports #15687

Merged
merged 2 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/codegen/bundle-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,16 @@ for (let i = 0; i < moduleList.length; i++) {
try {
let input = fs.readFileSync(path.join(BASE, moduleList[i]), "utf8");

// NOTE: internal modules are parsed as functions. They must use ESM to export and require to import.
// TODO: Bother @paperdave and have him check for module.exports, and create/return a module object if detected.
if (!/\bexport\s+(?:function|class|const|default|{)/.test(input)) {
if (input.includes("module.exports")) {
throw new Error("Cannot use CommonJS module.exports in ESM modules. Use `export default { ... }` instead.");
} else {
throw new Error("Internal modules must have an `export default` statement.");
}
}

const scannedImports = t.scanImports(input);
for (const imp of scannedImports) {
if (imp.kind === "import-statement") {
Expand Down Expand Up @@ -407,7 +417,7 @@ writeIfNotChanged(
// In this enum are represented as \`(1 << 9) & id\`
InternalModuleRegistryFlag = 1 << 9,
${moduleList.map((id, n) => ` ${idToEnumName(id)} = ${(1 << 9) | n},`).join("\n")}

// Native modules run through the same system, but with different underlying initializers.
// They also have bit 10 set to differentiate them from JS builtins.
NativeModuleFlag = (1 << 10) | (1 << 9),
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/internal-module-registry-scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export function createInternalModuleRegistry(basedir: string) {
const found = moduleList.indexOf(path.relative(basedir, relativeMatch).replaceAll("\\", "/"));
if (found === -1) {
throw new Error(
`Builtin Bundler: "${specifier}" cannot be imported here because it doesn't get a module ID. Only files in "src/js" besides "src/js/builtins" can be used here. Note that the 'node:' or 'bun:' prefix is required here. `,
`Builtin Bundler: "${specifier}" cannot be imported from "${from}" because it doesn't get a module ID. Only files in "src/js" besides "src/js/builtins" can be used here. Note that the 'node:' or 'bun:' prefix is required here. `,
);
}
return codegenRequireId(`${found}/*${path.relative(basedir, relativeMatch)}*/`);
Expand Down
Loading