taoqf / node-html-parser

A very fast HTML parser, generating a simplified DOM, with basic element query support.
MIT License
1.11k stars 107 forks source link

Fix css-select import #167

Closed rondonjon closed 2 years ago

rondonjon commented 2 years ago

May I suggest to change the css-select import in html.ts from

import { selectAll, selectOne } from 'css-select';

...

return selectAll( ...

to

import * as cssSelect from 'css-select';

...

return cssSelect.selectAll( ...

?

It seems that css-select is only exporting a CommonJS build, in which case the bundler/compiler must decide how to resolve the import. At least for Snowpack (and possibly Rollup, which Snowpack uses internally) this seems to cause problems.

Bundlers which follow the ESM specification strictly will resolve the import statement in the first snippet (version 5.0.0) to the default export of the CommonJS module, which is in this case the selectAll function of the css-select module. { selectAll, selectOne } are then handled as a destructuring assignment on the expected object, i.e. they will refer to selectAll.selectAll and selectAll.selectOne and resolve to undefined. This doesn't cause immediate problems, but will raise runtime errors when the functions are invoked later in the code.

The second snippet / suggested change refers explicitly to the "exported namespace" of the CommonJS module, so that selectAll and selectOne will resolve to the named exports (the functions) as intended.

rondonjon commented 2 years ago

Turns out the problem is caused by snowpack. Filing a new bug there.

lindseymacmillan commented 1 year ago

Hello! I know this issue was closed -- but I'm running into the same problem @rondonjon described, when using the plugin-node-resolve plugin for Rollup. It apparently uses the Node resolution algorithm to sort out dependencies and seems to have the exact same problem with css-select import as was raised in this issue. Curious if you've got any insight here, or specific concern about rewriting the commonJS imports.