ESM imports not working properly

[ivank]

Cannot be imported as an ESM module since the code dynamically loads a dependency via "require".

That require is still there in the "compiled" ES file https://www.npmjs.com/package/turndown?activeTab=code.

Line in question:

turndown/src/html-parser.js

Line 50 in 0df0c05

var domino = require('@mixmark-io/domino')

I was able to successfully use this by copy-pasting this whole file and rewriting the require to an import.

I understand that you don't want to add the dependency for browsers but this style of optional imports need to be rewritten properly with an import() function I guess.

Alternatively, you could specify the domino dependency as a peer dependency, and let people in the node environment supply it themselves.

[pavelhoral]

This is kind of hard to solve without changing the overall API (dynamic import means promises).

Although not ideal, the workaround for this is to supply already parsed DOM Node to the turndown function. When the input is not a string, no DOM parser will be loaded.

[Nipsuli]

Is there some reason why couldn't one just do like:

--- a/node_modules/turndown/lib/turndown.es.js
+++ b/node_modules/turndown/lib/turndown.es.js
@@ -576,11 +576,11 @@ function canParseHTMLNatively () {
   return canParse
 }
 
-function createHTMLParser () {
+async function createHTMLParser () {
   var Parser = function () {};
 
   {
-    var domino = require('@mixmark-io/domino');
+    var domino = await import('@mixmark-io/domino');
     Parser.prototype.parseFromString = function (string) {
       return domino.createDocument(string)
     };
@@ -588,7 +588,7 @@ function createHTMLParser () {
   return Parser
 }
 
-var HTMLParser = canParseHTMLNatively() ? root.DOMParser : createHTMLParser();
+var HTMLParser = canParseHTMLNatively() ? root.DOMParser : await createHTMLParser();
 
 function RootNode (input, options) {
   var root;

As top level await has been part of esm for few years already and in Node since 16?

[pavelhoral]

@Nipsuli That is an interesting idea. But I don't think it is as simple as this will break when transpiling to CJS and also the ESM build (i.e. the final bundle) would probably always import the dependency no matter what - and that is not what we want.

[ivank]

Although not ideal, the workaround for this is to supply already parsed DOM Node to the turndown function. When the input is not a string, no DOM parser will be loaded.

Sadly this is not a feasible workaround because the problem is on the compile side. The compiler we are forced to use (rollup) insists on not having any "require" statements in the code regardless if it is invoked or not

[pavelhoral]

The compiler we are forced to use (rollup) insists on not having any "require" statements in the code regardless if it is invoked or not

Maybe you can mark require as global variable?

Or you can remove the problematic statement with RollupJS transform plugin... something like:

  // rollup.config.js (using https://www.npmjs.com/package/magic-string to get correct source map)
  plugins: [
    {
      name: 'remove-require',
      transform(code, id) {
        const ms = new MagicString(code)
        ms.replaceAll(/require('@mixmark-io\/domino')/g, "null")
        return {
          code: ms.toString(),
          map: ms.generateMap({ hires: true })
        }
      }
    }
  ]

Of course I understand both approaches are not optimal. Ideally Turndown shouldn't have the Domino dependency at all...

[martincizek]

This would be solved when something like #290 is implemented. Splitting the project into two is a big change, though.

[domchristie]

Could we use different entry points and have a node build and a browser build as separate files? https://nodejs.org/api/packages.html#package-entry-points

[Nipsuli]

Maybe you can mark require as global variable

That is not valid solution. And rollup is correct on not allowing require be part of esm modules. As require is not part of ECMAScript standard but a Node specific thing. Example in Cloudflare Workers one does not have require and using this package will result in run time crash there.

And a version patched with the require -> import works correctly both on client side and on server side with react-router (remix) app on cloudflare workers.

[pavelhoral]

Example in Cloudflare Workers one does not have require and using this package will result in run time crash there.

I don't think you understand my message. The code that uses require should not be executed at all if you provide Turndown with already parsed DOM. I understand that require is not available in your environment, I was just suggesting various approaches how you can make your compiler / bundler not throw exception during build time.

And a version patched with the require -> import works correctly both on client side and on server side with react-router (remix) app on cloudflare workers.

Yes, it works in your use-case, not the use-cases I mentioned (i.e. CJS and ESM where you don't want Domino).

[anhkind]

@pavelhoral That's great idea just to patch that tricky dynamic require. This is what GPT-5 suggested me for the build with vite, and we don't really need MagicString at all:

plugins: [
  {
    name: 'replace-domino-require',
    enforce: 'pre',
    transform(code, id) {
      if (!id || !id.includes('node_modules/turndown')) return null;
      const pattern = /require\(['"]@mixmark-io\/domino['"]\)/g;
      if (pattern.test(code)) {
        const replaced = code.replace(pattern, 'undefined');
        return { code: replaced, map: null };
      }
      return null;
    },
  },
],

As long as we don't use string input for turndown function, this monkey patch is good enough.

[matijagrcic]

@pavelhoral That's great idea just to patch that tricky dynamic require. This is what GPT-5 suggested me for the build with vite, and we don't really need MagicString at all:

plugins: [
{
name: 'replace-domino-require',
enforce: 'pre',
transform(code, id) {
if (!id || !id.includes('node_modules/turndown')) return null;
const pattern = /require(['"]@mixmark-io/domino['"])/g;
if (pattern.test(code)) {
const replaced = code.replace(pattern, 'undefined');
return { code: replaced, map: null };
}
return null;
},
},
],
As long as we don't use string input for turndown function, this monkey patch is good enough.

Same as this #469 (comment)