mixmark-io / turndown

🛏 An HTML to Markdown converter written in JavaScript
https://mixmark-io.github.io/turndown
MIT License
8.84k stars 880 forks source link

With statements cannot be used with the "esm" output format due to strict mode #378

Open cheetahbyte opened 3 years ago

cheetahbyte commented 3 years ago
> node_modules/domino/lib/sloppy.js:10:4: error: With statements cannot be used with the "esm" output format due to strict mode
    10 │     with(this) eval(code);
       ╵     ~~~~

 > node_modules/domino/lib/sloppy.js:14:6: error: With statements cannot be used with the "esm" output format due to strict mode
    14 │       with(this.document.defaultView || Object.create(null))
       ╵       ~~~~

 > node_modules/domino/lib/sloppy.js:15:8: error: With statements cannot be used with the "esm" output format due to strict mode
    15 │         with(this.document)
       ╵         ~~~~

 > node_modules/domino/lib/sloppy.js:16:10: error: With statements cannot be used with the "esm" output format due to strict mode
    16 │           with(this.form)
       ╵           ~~~~

 > node_modules/domino/lib/sloppy.js:17:12: error: With statements cannot be used with the "esm" output format due to strict mode
    17 │             with(this.element)
       ╵             ~~~~

error when starting dev server:
Error: Build failed with 5 errors:
node_modules/domino/lib/sloppy.js:10:4: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:14:6: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:15:8: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:16:10: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:17:12: error: With statements cannot be used with the "esm" output format due to strict mode
    at failureErrorWithLog (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:1224:15)
    at buildResponseToResult (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:936:32)
    at C:\Users\Leonhard Breuer\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:1035:20
    at C:\Users\Leonhard Breuer\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:568:9
    at handleIncomingPacket (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:657:9)
    at Socket.readFromStdout (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:535:7)
    at Socket.emit (events.js:315:20)
    at addChunk (internal/streams/readable.js:309:12)
    at readableAddChunk (internal/streams/readable.js:284:9)
    at Socket.Readable.push (internal/streams/readable.js:223:10)
I get this error, how can I fix it?
logycware commented 3 years ago

Having the same problem

damianmozniak commented 3 years ago

Having the same problem

martincizek commented 3 years ago

There are some packaging changes in the master, so I'd like to ask you to try it first, i.e.:

  "dependencies": {
    "turndown": "github:domchristie/turndown"
  }

If the issue is still present, please provide a minimum buildable example reproducing the issue. Including:

Thank you!

callmeberzerker commented 3 years ago

Hi @martincizek

The entry points for "turndown": "github:domchristie/turndown" are wrong.

There is no lib folder like referenced here in package.json from master:

"main": "lib/turndown.cjs.js",
"module": "lib/turndown.es.js",
"jsnext:main": "lib/turndown.es.js",
damianmozniak commented 3 years ago

@martincizek solution works for me

callmeberzerker commented 3 years ago

@damianmozniak not sure how since there is no lib folder?

damianmozniak commented 3 years ago

@callmeberzerker lib exist after npm install so it's configured somewhere

martincizek commented 3 years ago

So it seems it was PR #335 what fixes this (merged but not released to npmjs).

We're currently in a process of transferring Turndown under a new organisation and I hope I'll be able to release a new npm package soon.

callmeberzerker commented 3 years ago

To anyone to whom this is a blocker and is using yarn as their package manager - I have published the latest master and the dist and lib folders on my forked version.

You would need to add the following to your dependencies:

"turndown": "https://github.com/callmeberzerker/turndown/commit/ede17b4aee0ee12c1615d65b28f64b9aa3e2b7e8"

Hopefully the proper version is released as soon as possible.

The maintainer published a temp branch see here: https://github.com/domchristie/turndown/issues/378#issuecomment-862390985

martincizek commented 3 years ago

To anyone to whom this is a blocker and is using yarn as their package manager - I have published the latest master and the dist and lib folders on my forked version.

@callmeberzerker Not sure how this differ from the current master. I see three commits in your fork:

  1. You've commited generated stuff, which is not necessary, because dependencies are built automatically when linked as a git dependency.
  2. You've introduced a mistake in package.json
  3. You've fixed your mistake from previous commit :-), which leads to the same package.json as in Turndown's master (except the script externalisation, which should be irrelevant for the functionality).

Are you sure that the result is any different from using just:

  "dependencies": {
    "turndown": "github:domchristie/turndown"
  }

?

P.S. Don't forget to npm install in your project, that's what also builds the turndown dependency.

callmeberzerker commented 3 years ago

Hi @martincizek

As I explained in previous comment, I am not using npm - but yarn (version 1) -> I basically need the published lib files irrelevant of my build/package manager. That's why I can't rely on "turndown": "github:domchristie/turndown" to be then run with npm to get the generated files.

martincizek commented 3 years ago

As I explained in previous comment, I am not using npm - but yarn (version 1) -> I basically need the published lib files irrelevant of my build/package manager.

I see. Not very familiar with yarn, I guess it's because of https://github.com/yarnpkg/yarn/issues/5235, right?

I've created a temporary branch with pre-built artifacts as a workaround until a new npm package is published. Would you mind to test it?

Workaround for yarn version 1:

  "dependencies": {
    "turndown": "github:domchristie/turndown#build-7.1"
  }
callmeberzerker commented 3 years ago

First of all, thanks a lot for your prompt feedback and communication!

I can definitely confirm that it works with your branch -> I will edit my above comment to point your comment.

I know the whole node/js ecosystem is hurting big now with the esm/module/main semi-standardization going on now which is hell for lib authors and users - but we will get through it somehow!

Once again thanks a lot!

cheetahbyte commented 3 years ago

Since my bug is fixed, and everyone else seems to be happy too, I'll close the issue :D

martincizek commented 3 years ago

@callmeberzerker @cheetahbyte Release 7.1 has been created and published to npm registry. Would you mind to test it, especially with yarn?

It should now be just:

  "dependencies": {
    "turndown": "^7.1.0"
  }
jb-modist commented 2 years ago

I created the patch file:

diff --git a/node_modules/domino/lib/sloppy.js b/node_modules/domino/lib/sloppy.js

index b5d8950..e920db1 100644
--- a/node_modules/domino/lib/sloppy.js
+++ b/node_modules/domino/lib/sloppy.js
@@ -6,19 +6,9 @@
 /* jshint -W085 */
 module.exports = {
   Window_run: function _run(code, file) {
-    if (file) code += '\n//@ sourceURL=' + file;
-    with(this) eval(code);
+    console.log("Window_run removed")
   },
   EventHandlerBuilder_build: function build() {
-    try {
-      with(this.document.defaultView || Object.create(null))
-        with(this.document)
-          with(this.form)
-            with(this.element)
-              return eval("(function(event){" + this.body + "})");
-    }
-    catch (err) {
-      return function() { throw err; };
-    }
+    console.log("EventHandlerBuilder_build removed")
   }
 };

which I deploy to the server by adding "postinstall": "patch-package"

as a package.json script.

This solved the problem for me.

Weyker commented 2 years ago

same error Version: 7.1.1

WenyaoL commented 1 year ago

In html-parser.js

var domino = require('domino')
Parser.prototype.parseFromString = function (string) {
    return domino.createDocument(string)
}

to

import('domain').then(domino=>{
Parser.prototype.parseFromString = function (string) {
      return domino.createDocument(string)
    }
})
julienc91 commented 1 year ago

Hi, I encountered the same error when I tried to migrate my nextjs app to use their new app router system. The problem appears when turndown is imported anywhere from the app directory. I created this repository to easily reproduce the error: https://github.com/julienc91/turndown-strict-mode

Karalix commented 1 year ago

Hi, I upgraded a bunch of dependencies in my Nuxt project and this bug broke the Netlify build with this message require() of ES Module /var/task/.netlify/functions-internal/server nuxt/server.mjs not supported.

I was using turndown 7.1.2

Removing turndown from my project fixed the deployment

Growmedigitally commented 1 year ago

× With statement are not allowed in strict mode ╭─[/Users/danny/Documents/Target/Projects/node_modules/domino/lib/sloppy.js:13:1] 13 │ try { 14 │ with(this.document.defaultView || Object.create(null)) 15 │ with(this.document) 16 │ with(this.form) · ──── 17 │ with(this.element) 18 │ return eval("(function(event){" + this.body + "})"); 19 │ } ╰────

Does anyone find the solution for this ?? 

nextjs app : 13.5.4
turndown :

tried turndown :7.1.2 turndown :7.1.1 "turndown": "github:domchristie/turndown" "turndown": "github:domchristie/turndown#build-7.1"

    but still issue present
Growmedigitally commented 1 year ago

Hi, I encountered the same error when I tried to migrate my nextjs app to use their new app router system. The problem appears when turndown is imported anywhere from the app directory. I created this repository to easily reproduce the error: https://github.com/julienc91/turndown-strict-mode

have you found any solution?

Growmedigitally commented 1 year ago
> node_modules/domino/lib/sloppy.js:10:4: error: With statements cannot be used with the "esm" output format due to strict mode
    10 │     with(this) eval(code);
       ╵     ~~~~

 > node_modules/domino/lib/sloppy.js:14:6: error: With statements cannot be used with the "esm" output format due to strict mode
    14 │       with(this.document.defaultView || Object.create(null))
       ╵       ~~~~

 > node_modules/domino/lib/sloppy.js:15:8: error: With statements cannot be used with the "esm" output format due to strict mode
    15 │         with(this.document)
       ╵         ~~~~

 > node_modules/domino/lib/sloppy.js:16:10: error: With statements cannot be used with the "esm" output format due to strict mode
    16 │           with(this.form)
       ╵           ~~~~

 > node_modules/domino/lib/sloppy.js:17:12: error: With statements cannot be used with the "esm" output format due to strict mode
    17 │             with(this.element)
       ╵             ~~~~

error when starting dev server:
Error: Build failed with 5 errors:
node_modules/domino/lib/sloppy.js:10:4: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:14:6: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:15:8: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:16:10: error: With statements cannot be used with the "esm" output format due to strict mode
node_modules/domino/lib/sloppy.js:17:12: error: With statements cannot be used with the "esm" output format due to strict mode
    at failureErrorWithLog (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:1224:15)
    at buildResponseToResult (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:936:32)
    at C:\Users\Leonhard Breuer\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:1035:20
    at C:\Users\Leonhard Breuer\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:568:9
    at handleIncomingPacket (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:657:9)
    at Socket.readFromStdout (C:\Users\user\AppData\Roaming\npm\node_modules\vite\node_modules\esbuild\lib\main.js:535:7)
    at Socket.emit (events.js:315:20)
    at addChunk (internal/streams/readable.js:309:12)
    at readableAddChunk (internal/streams/readable.js:284:9)
    at Socket.Readable.push (internal/streams/readable.js:223:10)
I get this error, how can I fix it?

have you find any solution?

julienc91 commented 1 year ago

@Growmedigitally Not really fixed, but I managed to work around the problem by using JSDOM to bypass domino.

You can have a look at this commit here: https://github.com/julienc91/caviardeul/commit/75570c15a0db15e5c523a3b6b86773f04c594cf2

Growmedigitally commented 1 year ago

@Growmedigitally Not really fixed, but I managed to work around the problem by using JSDOM to bypass domino.

You can have a look at this commit here: julienc91/caviardeul@75570c1

Still not working same issue { "dependencies": {

.... "jsdom": "^22.1.0", "turndown": "^7.1.2" }, "devDependencies": { "@types/jsdom": "^21.1.3", "@types/turndown": "^5.0.2", } }

ioRekz commented 1 year ago

also have the same issue when working with nextjs. Would it be possible to replace domino with JSDOM similar to what @julienc91 did in this repo?

I can't do it since I'm not depending directly on turndown but postlight/parser -> turndown -> domino :(

headline-design commented 11 months ago

Next14 issue related to this here - https://github.com/vercel/next.js/issues/34796

LuisGilGB commented 11 months ago

Sandbox to reproduce the error when using Next JS 14.0.2 or 14.0.3 (for 14.0.1 or older it seems to build successfully): https://codesandbox.io/p/devbox/next-14-0-2-domino-build-error-bug-dylymw (I encourage downloading the app and running locally, as the resources are quite constrained).

As said in earlier comments, this is an issue with domino, not with turndown. In this sandbox app, turndown is required as just a dependency of remirror, the module defined as a dependency of the app (even not using the markdown string handler, which is what actually needs turndown).

datduyng commented 8 months ago

Adding below snippets in package.json works fix the issue for me

  "resolutions": {
    "domino": "Haringat/domino#2721294c75334ece635dfc72cad0a3dede7391fb"
  }

CleanShot 2024-02-08 at 20 58 54

zirkelc commented 7 months ago

The package https://github.com/fgnass/domino that causes this error seems to be no longer maintained, so there is no hope that it will be fixed. If a DOM is needed from this package, perhaps https://github.com/capricorn86/happy-dom could be a lightweight alternative to JSDOM? I'd love to help and test it, but I'd like to know if there's any interest in getting rid of this problem at all?

In the meantime, I've switched to https://github.com/crosstype/node-html-markdown as a workaround. It's pretty basic and also not up to date, but at least it works on AWS Lambda.

martincizek commented 7 months ago

Hi @zirkelc,

perhaps https://github.com/capricorn86/happy-dom could be a lightweight alternative to JSDOM? I'd love to help and test it, but I'd like to know if there's any interest in getting rid of this problem at all?

Any meaningful help is appreciated. For the final solution, I'd like the approach described in #290. But the domino is a current issue, so we shouldn't wait. We are actually in the same situation as with #353 back then. :-)

So if you can try another parser, test it and do some perf tests, it would be nice. Alternatively, we can at least temporarily adopt this fork https://github.com/angular/domino, which might have the issues fixed (?) It is not intended for use as a public package, but we can fork it under mixmark-io and use it in a similar way as Angular uses it.

zirkelc commented 7 months ago

Hi @martincizek

I submitted PR https://github.com/mixmark-io/turndown/pull/457 just to share some results. I added benchmark and happy-dom is unfortunately a lot slower than domino. I also added the html parser parse5 briefly, but its Document interface doesn't implement function like getElementByID. It's really only meant for parsing HTML into a list of nodes.

Maybe there are other dies for parser that could be added to compare with domino.

EDIT: I added jsdom for comparison as well.

yagudaev commented 6 months ago

Thanks so much for this 💜.

The angular fork does solve the issue.

You can solve it by using:

"resolutions": {
  "domino": "angular/domino#8f228f8862540c6ccd14f76b5a1d9bb5458618af"
}

Successfully deployed and working on Vercel.

@datduyng 's repo he found, didn't quite work. But provided the needed inspiration for the workaround 🙏