fix: support esbuild by lisonge · Pull Request #166 · taoqf/node-html-parser

lisonge · 2021-10-18T06:06:15Z

fix the following bug

npm i esbuild node-html-parser typescript
echo 'import { parse, HTMLElement } from "node-html-parser";console.log({ parse, HTMLElement });' > test.ts
npx esbuild test.ts --bundle --outfile=test.js
node test.js

will be output { parse:undefined, HTMLElement:undefined }

nonara · 2021-10-19T01:46:39Z

Thanks for the submission!

In order to use as esm your package.json file needs to have "type": "module" set. Please let me know if that solves the issue.

lisonge · 2021-10-19T03:09:12Z

your packge default export is https://github.com/taoqf/node-html-parser/blob/main/src/index.ts#L3 is a function

export { default as parse, default } from './parse';

but in https://github.com/taoqf/node-html-parser/blob/main/esm/index.js#L1

import nhp from '../dist/index.js'

and at dist/index.js#L5

Object.defineProperty(exports, "__esModule", { value: true });

and at #L13

Object.defineProperty(exports, "default", { enumerable: true, get: function () { return __importDefault(parse_1).default; } });

so esbuild will use module.exports.default as import nhp from '../dist/index.js'

so nhp is parse function, not module.exports

lisonge · 2021-10-19T15:26:25Z

Thanks for the submission!

In order to use as esm your package.json file needs to have "type": "module" set. Please let me know if that solves the issue.

I need run node-html-parser in cloudflare workers, without package.json, it is not node runtime
so I use esbuild pack my code to a single file. currently I fix this with patches in my project
@nonara

nonara · 2021-10-19T16:34:44Z

your packge default export is https://github.com/taoqf/node-html-parser/blob/main/src/index.ts#L3 is a function

The issue is that package exports are handled differently between ESM and CommonJS.

import nhp from 'node-html-parser' in CommonJS, means import module.exports.default and name it nhp.

In ESM, the same statement means: import the entire module namespace and name it nhp.

Our structure for ESM support follows the proper convention and is not buggy. You can see more detail in this PR:

Multiple fixes #161

I need run node-html-parser in cloudflare workers, without package.json

It looks like you don't actually need to be using ESM. If you build to commonJS, you should be fine.

I tried your exact reproduction, and I don't actually experience any issue:

npm i esbuild node-html-parser typescript
echo 'import { parse, HTMLElement } from "node-html-parser";console.log({ parse, HTMLElement });' > test.ts
npx esbuild test.ts --bundle --outfile=test.js
node test.js
{
  parse: [Function: parse],
  HTMLElement: [class HTMLElement extends Node]
}

Do you have a tsconfig.json you can share? If you can setup a repository which can reproduce the issue you're having, I'll take a look and see if I can help.

lisonge · 2021-10-20T02:06:44Z

Do you have a tsconfig.json you can share? If you can setup a repository which can reproduce the issue you're having, I'll take a look and see if I can help.

tsconfig.json in my project https://github.com/lisonge/visit-counter.git

this project will show a bug, If you don't modify node_modules/node-html-parser/esm/index.js

you see bug by following shell

npx esbuild test/test_import.ts --bundle --outfile=dist/test_import.js
node dist/test_import.js

lisonge · 2021-10-20T02:31:46Z

I tried your exact reproduction, and I don't actually experience any issue:

my shell Log

❯ cat typescript
Script started on 2021-10-20 10:26:37+08:00 [TERM="xterm-256color" TTY="/dev/pts/0" COLUMNS="120" LINES="30"]
❯ ls -a .
.  ..  install.sh  typescript
❯ cat install.sh
npm i esbuild node-html-parser typescript
echo 'import { parse, HTMLElement } from "node-html-parser";console.log({ parse, HTMLElement });' > test.ts
npx esbuild test.ts --bundle --outfile=test.js
node test.js

❯ ./install.sh

> esbuild@0.13.8 postinstall /home/lisonge/project/test/node_modules/esbuild
> node install.js

npm WARN saveError ENOENT: no such file or directory, open '/home/lisonge/project/test/package.json'
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-freebsd-64@0.13.8 (node_modules/esbuild/node_modules/esbuild-freebsd-64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-freebsd-64@0.13.8: wanted {"os":"freebsd","arch":"x64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-freebsd-arm64@0.13.8 (node_modules/esbuild/node_modules/esbuild-freebsd-arm64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-freebsd-arm64@0.13.8: wanted {"os":"freebsd","arch":"arm64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-linux-arm@0.13.8 (node_modules/esbuild/node_modules/esbuild-linux-arm):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-linux-arm@0.13.8: wanted {"os":"linux","arch":"arm"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-darwin-64@0.13.8 (node_modules/esbuild/node_modules/esbuild-darwin-64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-darwin-64@0.13.8: wanted {"os":"darwin","arch":"x64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-android-arm64@0.13.8 (node_modules/esbuild/node_modules/esbuild-android-arm64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-android-arm64@0.13.8: wanted {"os":"android","arch":"arm64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-linux-32@0.13.8 (node_modules/esbuild/node_modules/esbuild-linux-32):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-linux-32@0.13.8: wanted {"os":"linux","arch":"ia32"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-linux-mips64le@0.13.8 (node_modules/esbuild/node_modules/esbuild-linux-mips64le):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-linux-mips64le@0.13.8: wanted {"os":"linux","arch":"mips64el"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-darwin-arm64@0.13.8 (node_modules/esbuild/node_modules/esbuild-darwin-arm64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-darwin-arm64@0.13.8: wanted {"os":"darwin","arch":"arm64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-linux-arm64@0.13.8 (node_modules/esbuild/node_modules/esbuild-linux-arm64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-linux-arm64@0.13.8: wanted {"os":"linux","arch":"arm64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-linux-ppc64le@0.13.8 (node_modules/esbuild/node_modules/esbuild-linux-ppc64le):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-linux-ppc64le@0.13.8: wanted {"os":"linux","arch":"ppc64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-netbsd-64@0.13.8 (node_modules/esbuild/node_modules/esbuild-netbsd-64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-netbsd-64@0.13.8: wanted {"os":"netbsd","arch":"x64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-openbsd-64@0.13.8 (node_modules/esbuild/node_modules/esbuild-openbsd-64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-openbsd-64@0.13.8: wanted {"os":"openbsd","arch":"x64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-sunos-64@0.13.8 (node_modules/esbuild/node_modules/esbuild-sunos-64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-sunos-64@0.13.8: wanted {"os":"sunos","arch":"x64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-windows-32@0.13.8 (node_modules/esbuild/node_modules/esbuild-windows-32):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-windows-32@0.13.8: wanted {"os":"win32","arch":"ia32"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-windows-64@0.13.8 (node_modules/esbuild/node_modules/esbuild-windows-64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-windows-64@0.13.8: wanted {"os":"win32","arch":"x64"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: esbuild-windows-arm64@0.13.8 (node_modules/esbuild/node_modules/esbuild-windows-arm64):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for esbuild-windows-arm64@0.13.8: wanted {"os":"win32","arch":"arm64"} (current: {"os":"linux","arch":"x64"})
npm WARN enoent ENOENT: no such file or directory, open '/home/lisonge/project/test/package.json'
npm WARN test No description
npm WARN test No repository field.
npm WARN test No README data
npm WARN test No license field.

+ node-html-parser@5.0.0
+ typescript@4.4.4
+ esbuild@0.13.8
added 14 packages from 7 contributors and audited 30 packages in 2.238s

8 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities



   ╭───────────────────────────────────────────────────────────────╮
   │                                                               │
   │      New major version of npm available! 6.14.15 → 8.1.0      │
   │   Changelog: https://github.com/npm/cli/releases/tag/v8.1.0   │
   │               Run npm install -g npm to update!               │
   │                                                               │
   ╰───────────────────────────────────────────────────────────────╯


  test.js  322.4kb

⚡ Done in 18ms
{ parse: undefined, HTMLElement: undefined }
❯ exit

Script done on 2021-10-20 10:27:04+08:00 [COMMAND_EXIT_CODE="0"]

test.js here https://gist.github.com/lisonge/bfb21198fa027fa285e7016c0672abbf#file-test-js-L5086

lisonge · 2021-10-20T02:44:46Z

It looks like you don't actually need to be using ESM. If you build to commonJS, you should be fine.

i try --platform=node, but same error

❯ npx esbuild test.ts --bundle --outfile=test.js --platform=node

  test.js  313.1kb

⚡ Done in 19ms
❯ node test.js
{ parse: undefined, HTMLElement: undefined }

nonara · 2021-10-20T21:17:23Z

Ok. I cloned your repo and had a look. A few observations:

Something is going wrong with your build
Looks like you don't need to use --platform=node

It works for me with both platform=node and otherwise, using your exact repository.

$ npx esbuild test/test_import.ts --bundle --outfile=dist/test_import.js

  dist\test_import.js  300.2kb

Done in 55ms

$ node dist/test_import.js
{
  parse: [Function: parse],
  HTMLElement: [class HTMLElement extends Node]
}

$ npx esbuild test/test_import.ts --bundle --outfile=dist/test_import.js --platform=node

  dist\test_import.js  310.9kb

Done in 115ms

$ node dist/test_import.js
{ parse: [Function: parse2], HTMLElement: [Function: HTMLElement3] }

I recommend the following:

Uninstall your global version of esbuild (if one is installed)
Ensure pnpm cache is cleared (global and local, if applicable)
Delete pnpm-lock.yaml in your repo
Make sure your package.json specifies the latest version of esbuild
Run pnpm install
Directly execute esbuild from node_modules (without npx) and check the output

If there is an issue beyond that, it's with esbuild. I've confirmed and it's pulling in the proper source code from the commonJS location, so this is not an issue with our ESM wrapper.

Good luck with it! I hope you get it sorted out.

lisonge · 2021-10-21T01:27:33Z

i try run shell code at github actions ubuntu-latest
https://github.com/lisonge/test-cli/runs/3958824691?check_suite_focus=true#step:10:1
it still output

{ parse: undefined, HTMLElement: undefined }

can you show dist/test_import.js on your computer?

lisonge · 2021-10-25T11:14:49Z

@nonara
hello, can you show dist/test_import.js on your computer to me?

nonara · 2021-10-25T17:05:48Z

Here's my file: https://gist.github.com/nonara/67ef1505a98a2c9ad02bf4ab06f0b183

I'd love to be able to help you track this out, but unfortunately I'm swamped. With that said, hopefully this will help point you in the right direction:

It builds properly for me with both --platform=node and without — on Windows
Looking at your output and mine shows that esbuild is failing somewhere
This is probably related to how esbuild handles the exports field, as the comment output is also wrong:

Note that even though it works for me, the comment still says this:
// ../node_modules/node-html-parser/dist/esm/nodes/node.js

^ That is not an actual file path. This suggests that esbuild isn't handling the package.sjon exports field correctly, which is a common issue. Handling ESM is not easy, and many libraries break down when given ESM. However, in this case, esbuild should be importing directly from commonjs, which it appears that it is for me. It looks like it is for you, but it's still somehow breaking.

Given the discrepancy between your build failing and mine not, my best guess is that there is an issue between windows and unix.

Recommended steps:

Try running GH actions using windows as your OS. If it works, you'll know it's related to OS
When you've gathered more info, file a ticket with esbuild and show them the output

Again, I've confirmed that this is not an issue with our esm wrapper, as we followed the proper convention, and it works in both commonjs and esm environments.

Good luck with it, and I hope you get it sorted out!

lisonge · 2021-10-26T08:11:58Z

I think the reason should be this

because https://github.com/taoqf/node-html-parser/blob/main/tsconfig.json#L24

"esModuleInterop": true,

generate https://cdn.jsdelivr.net/npm/node-html-parser@5.0.0/dist/index.js#L5

Object.defineProperty(exports, "__esModule", { value: true });

this line code mark current file is cjs that compiled from esm, it can help keep connection between es modules in cjs runtime

when esm import cjs in nodejs runtime, it doesn't depend on __esModule property,
so it will use exports instead of exports['default']

but esbuild will depend on __esModule property , so it get exports['default']

and when i change Object.defineProperty(exports, "__esModule", { value: true }); to Object.defineProperty(exports, "__esModule", { value: false }); , the [code by esbuild pack] is work

for your this

Note that even though it works for me, the comment still says this:
// ../node_modules/node-html-parser/dist/esm/nodes/node.js

I can't reproduce it at the moment, so i do not know

nonara · 2021-10-26T12:30:23Z

This is standard output. File an issue with esbuild.

fix: support esbuild

e715ced

fix: esm should use named exports

b80c610

lisonge added 2 commits October 19, 2021 23:31

update: restore

5f52dc6

fix: support esbuild

2f04c97

lisonge closed this Oct 20, 2021

lisonge reopened this Oct 20, 2021

nonara closed this Oct 20, 2021

lisonge mentioned this pull request Oct 27, 2021

esm import cjs in third-party-package will get exports.default instead of module.exports evanw/esbuild#1719

Closed

lisonge mentioned this pull request Nov 5, 2021

parse4 is not a function lisonge/visit-counter#1

Closed

nonara mentioned this pull request Nov 16, 2021

Fix non-default ESM imports #175

Closed

taoqf added a commit that referenced this pull request Aug 12, 2022

fix issue #208 #166

4d8beeb

taoqf mentioned this pull request Aug 12, 2022

fix: fix vite build error #208

Closed

Conversation

lisonge commented Oct 18, 2021

Uh oh!

nonara commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nonara commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nonara commented Oct 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lisonge commented Oct 25, 2021

Uh oh!

nonara commented Oct 25, 2021

Uh oh!

lisonge commented Oct 26, 2021

Uh oh!

nonara commented Oct 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nonara commented Oct 19, 2021 •

edited

Loading

lisonge commented Oct 19, 2021 •

edited

Loading

lisonge commented Oct 19, 2021 •

edited

Loading

nonara commented Oct 19, 2021 •

edited

Loading

lisonge commented Oct 20, 2021 •

edited

Loading

lisonge commented Oct 20, 2021 •

edited

Loading

lisonge commented Oct 20, 2021 •

edited

Loading

nonara commented Oct 20, 2021 •

edited

Loading

lisonge commented Oct 21, 2021 •

edited

Loading