Skip to content

fix: load CLI configs with top-level await - #4100

Open
pengboyu-dev wants to merge 1 commit into
markedjs:masterfrom
pengboyu-dev:fix/cli-async-esm-config
Open

pengboyu-dev wants to merge 1 commit into
markedjs:masterfrom
pengboyu-dev:fix/cli-async-esm-config

Conversation

@pengboyu-dev

Copy link
Copy Markdown

The CLI cannot load an ESM config containing top-level await on Node.js versions that support synchronous require(esm). For example:

// config.mjs
export default await Promise.resolve({ breaks: true });

marked --config config.mjs --string text exits with ERR_REQUIRE_ASYNC_MODULE on Node 24.19.0. The config loader only falls back to import() for ERR_REQUIRE_ESM, so it never reaches the existing asynchronous import path.

Treat ERR_REQUIRE_ASYNC_MODULE as another reason to use that path. Other loading errors continue to propagate, and the existing JSON/CommonJS loading path is unchanged.

Validation:

  • Added a CLI regression test using a config with top-level await; it fails before the fix and passes after it.
  • Full npm test passes on Node 24.19.0, including specs, unit tests, UMD/CJS, types, and lint.

Contributor

  • Tests exist to ensure functionality and minimize regression.
  • No tests required for this PR.
  • If submitting a new feature, it has been documented in the appropriate places.

@vercel

vercel Bot commented Sep 19, 2026

Copy link
Copy Markdown

@pengboyu-dev is attempting to deploy a commit to the MarkedJS Team on Vercel.

A member of the Team first needs to authorize it.

@vercel

vercel Bot commented Sep 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
marked-website Ready Ready Preview Sep 20, 2026 12:22am UTC

Request Review

@UziTech UziTech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! 💯

Comment thread bin/main.js
@@ -180,7 +180,7 @@ export async function main(nodeProcess) {
// try require for json
markedConfig = require(configFile);

@styfle styfle Sep 22, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove require(configFile) now since we can always run await import(pathToFileURL(configFile).href) with any LTS version of Node.js, and it works with ESM and CJS

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Agreed that dynamic import() can handle ordinary ESM and CJS configs. I checked the existing loader behavior and found two compatibility details we'd need to preserve:

  • The CLI documents and automatically loads ~/.marked.json. A plain import() fails for JSON unless we handle it explicitly or supply the appropriate import attribute.
  • CJS configs such as exports.default = { breaks: true } currently work. With import(), the additional namespace wrapper means the existing single .default unwrap no longer reaches the config; the CLI exits successfully but the option is not applied.

I verified the current patch across Node 20, 22, 24, and 26: it fixes configs with top-level await while preserving those existing cases. My preference would be to keep this PR as the targeted fix and handle the loader simplification separately with compatibility tests. Does that sound reasonable?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM @styfle thoughts?

This branch was successfully deployed

1 active deployment
Preview — 739748fa Deployed Sep 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants