ES6 Import doesn't work for me

[samw1989]

Hello, very new to Javascript, so apologies if I'm just misunderstanding and this isn't a bug.

Per the documentation, I ran:

import { extract } from 'article-parser'

This doesn't work for me. It leads to:

SyntaxError: Named export 'extract' not found. The requested module 'article-parser' is a CommonJS module, which may not support all module.exports as named exports.

Running import main from 'article-parser', and then calling main.extract() does work.

[ndaidong]

@samw1989 hello, thank you for your report. Which runtime/platform are you working on?

[samw1989]

Node.js (v15).

[ndaidong]

@samw1989 do you use TypeScript or esm, or run your server with –experimental-modules flag?
By default, the latest versions of Node.js do not support native ES6 module import/export syntax yet, unless you add type: "module" in your package.json

[samw1989]

Sorry for delayed response. Yes, running modular system using "type": "module".

I tried switching to CommonJS and require() works just fine. Only the ES6 import that is causing me trouble.

[ndaidong]

@samw1989 I will try to rewrite to use es6 module syntax instead of commonjs.

[SettingDust]

@samw1989 I will try to rewrite to use es6 module syntax instead of commonjs.

https://github.com/SettingDust/article-parser Something I did for es6 xd. But not for current version.
Whats the problem of the porting to es6 currently?

[ndaidong]

@SettingDust last month, I've created branch es6-module-format, and could not pass the test. The problem occurred with jsdom module.

3999e81

Do you have any suggestion?

[SettingDust]

Do you have any suggestion?

I've not runned into error

Test Suites: 14 passed, 14 total
Tests:       70 passed, 70 total
Snapshots:   0 total
Time:        8.662 s, estimated 14 s
Ran all test suites.

[ndaidong]

@SettingDust I see that your fork is quite far from my current version and you had added a lot of modifications, so they could not be merged easily.

[SettingDust]

@SettingDust I see that your fork is quite far from my current version and you had added a lot of modifications, so they could not be merged easily.

I mean your branch not run into error on my end
Edit:
Gotcha. Error when building.

[ndaidong]

@SettingDust at my local environment, it works as well too :)
It just doesn't pass the github CI. Anw, I will investigate the root cause here.

[SettingDust]

Just npm i canvas. But canvas is depends on native module which esbuild not support. Maybe need switch to webpack or rollup or parcel etc.

[ndaidong]

@SettingDust if so, rollup may help. Let's me try with Rollup to see if it works for us.

[SettingDust]

@SettingDust if so, rollup may help. Let's me try with Rollup to see if it works for us.

I'm trying to use rollup for building now

[SettingDust]

Just npm i canvas. But canvas is depends on native module which esbuild not support. Maybe need switch to webpack or rollup or parcel etc.

It's seems that we won't use the canvas? Maybe we should exclude it from building. So that esbuild can work

[ndaidong]

Thank you for your notice. I'm not sure if canvas is being used. My idea is:

• standard es6 module format as default, without compiling, just import and use. So we don't care about canvas.
• generated commonjs version for the rest, because many projects are being stuck at the outdated Node.js environments, not easy to upgrade.

[SettingDust]

Thank you for your notice. I'm not sure if canvas is being used. My idea is:

* standard es6 module format as default, without compiling, just import and use. So we don't care about `canvas`.

* generated commonjs version for the rest, because many projects are being stuck at the outdated Node.js environments, not easy to upgrade.

canvas is a native module. So there is a file which named canvas.node can't be bundled.

[ndaidong]

@SettingDust I think canvas is only available in browser environment, not native at Node.js platform yet. jsdom wants to mimic some browser behaviors, so it needs something to simulate HTML Canvas API.

[SettingDust]

@SettingDust I think canvas is only available in browser environment, not native at Node.js platform yet. jsdom wants to mimic some browser behaviors, so it needs something to simulate HTML Canvas API.

Yes, it's that. So, for user who want to parse canvas as well. They should install it in their project I think

[ndaidong]

@SettingDust It seems we have to wait for a while, until ES6 Module becomes more regular. More dependencies, more complicate to migrate. I've done with several public repos here, but sometimes these changes also cause unnecessary troubles!

[SettingDust]

@SettingDust It seems we have to wait for a while, until ES6 Module becomes more regular. More dependencies, more complicate to migrate. I've done with several public repos here, but sometimes these changes also cause unnecessary troubles!

In fact, the main branch depends on jsdom -> canvas, too. Just the esbuild want to parse the canvas. If set it as external. It's totally as same as main branch behavior (users need to install canvas themselves)

[SettingDust]

It's ok now. #212

[ndaidong]

@SettingDust cool! I will merge, but keep it as rc version for right now.
standard will be installed with CI config, so we don't need to have it as devDependencies.

[SettingDust]

@SettingDust cool! I will merge, but keep it as rc version for right now. standard will be installed with CI config, so we don't need to have it as devDependencies.

Run with npx needn't to install global in local and ci

[ndaidong]

@SettingDust I'm testing at local environment. ES6 version works as well, however commonjs version doesn't work yet!

ES6 evaluation:

CommonJS version:

[SettingDust]

@SettingDust I'm testing at local environment. ES6 version works as well, however commonjs version doesn't work yet!

ES6 evaluation:

CommonJS version:

Caused by esbuild not support dynamic import from https://github.com/ndaidong/article-parser/blob/main/src/utils/chooseBestUrl.js#L18
Releate: evanw/esbuild#700

[ndaidong]

@SettingDust thank you, if so, I think we should keep it there, the issue will be fixed by itself 😄

[SettingDust]

@SettingDust thank you, if so, I think we should keep it there, the issue will be fixed by itself 😄

Maybe we can disable the string comparison algorithm option temporary? Publish a version like 6.0.0-alpha

[ndaidong]

@SettingDust string comparison is needed to predict the best URL of article (with it we can normalize the images path too). I consider the trade-off to be not worth it.

[SettingDust]

Needn't remove entire module. Just disable the option to change algorithm. Use the default levenshtein.

[ndaidong]

@SettingDust I've merge you pr #217 , let's check if it works for us in several more tests.

[SettingDust]

@SettingDust I've merge you pr #217 , let's check if it works for us in several more tests.

Haven't repair the cjs dist issue

[ndaidong]

@SettingDust yes, while using cjs version, I face problem with string-comparison:

I found a workaround by copying a part of this lib to dist folder.

[ndaidong]

@SettingDust I just released v6.0.0-rc1, let's give it a try. When we feel it's reliable enough, it will be merged into main branch.

[SettingDust]

@SettingDust I just released v6.0.0-rc1, let's give it a try. When we feel it's reliable enough, it will be merged into main branch.

It's not caused by the dynamic import, MB. I'm looking into the esbuild

[SettingDust]

Weird. It's can build without any changes. Maybe it is fixed by itself. It's seems that a "Quantum" lul.
I read string-comparison's code https://github.com/Rabbitzzc/js-string-comparison/blob/master/lib/index.js. The modules aren't import with main/packages/${module}. But what we meetting says it use a dynamic import.
But, I noticed that Rabbitzzc/js-string-comparison@8eafa55. This commit remove the dynamic import part.
How dare esbuild build with an older version? Then I try to clear the package and npm i again. Well, it's fine now.

[SettingDust]

BTW, yarn not support version like x.x.xrc1. It's should be x.x.x-rc1.

Note: There are also sometimes “labels” or “extensions” to the semver format that mark things like pre-releases or betas (e.g. 2.0.0-beta.3)

[ndaidong]

@SettingDust thank you for pointing out, I will merge your latest pr now.

[ndaidong]

@SettingDust I don't know why but without my workaround, the issue is still happening

Could you try there step:

npm run reset  # to clean everything
npm i

# temporarily remove `"type": "module"` within package.json, then
npm run eval:cjs https://www.detroitnews.com/story/news/local/detroit-city/2022/02/13/ambassador-bridge-protesters-arrests-blockade-border/6775095001/

[SettingDust]

@SettingDust I don't know why but without my workaround, the issue is still happening

Could you try there step:

npm run reset  # to clean everything
npm i

# temporarily remove `"type": "module"` within package.json, then
npm run eval:cjs https://www.detroitnews.com/story/news/local/detroit-city/2022/02/13/ambassador-bridge-protesters-arrests-blockade-border/6775095001/

He havent published his changes with a tag. We need use the github source for package.

[ndaidong]

@SettingDust ok, I'm considering to change string comparison tool. The problem and code base of js-string-comparison show that it is not a reliable dependency anymore.

[ndaidong]

@SettingDust v6.0.0rc2 with your fix has been released, I see it works well now.

[Rabbitzzc]

my bad

[Rabbitzzc]

I published the version 1.1.0, please update to this version and try it.

[ndaidong]

@Rabbitzzc thank you so much! I'm worrying you've forgot it.

[ndaidong]

@SettingDust v6.0.0rc4 has been released. If nothing wrong, we can merge to main branch next week.