gajus / surgeon

Declarative DOM extraction expression evaluator. 👨‍⚕️
Other
695 stars 30 forks source link

webpack @babel/preset-env module build failed error in import statement #25

Closed ComLock closed 6 years ago

ComLock commented 6 years ago

https://github.com/gajus/surgeon/blob/master/src/evaluators/cheerioEvaluator.js#L4

Shouldn't this:

import type {
  EvaluatorType
} from '../types';

Be like this?:

import {EvaluatorType} from '../types';

You don't use type anywhere, and if you did there is a comma missing in the import statement.

See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import

gajus commented 6 years ago

All is correct.

https://flow.org/en/docs/types/modules/

gajus commented 6 years ago

Whats the context of the error?

./dist code does not include the referenced code.

ComLock commented 6 years ago

The context is the place I actually use the scraper.

Where I do as documented:

import {
  cheerioEvaluator
} from './evaluators';

surgeon({
  evaluator: cheerioEvaluator()
});

That import statement fails. My webpack/babel setup doesn't include flow. https://github.com/ComLock/enonic-xp-app-crawler/blob/master/webpack.config.babel.js#L83

Even with flow, according to the documentation you linked, shouldn't it be like this?

import type Somename, {
  EvaluatorType
} from '../types';
gajus commented 6 years ago

I don't see surgeon in the package.json. How are you consuming the library?

The distributed code does not include Flow types.

https://github.com/gajus/surgeon/blob/master/.babelrc#L11

If you are not using Flow, then you don't need to import types.

ComLock commented 6 years ago

I have not pushed the code yet. I can mention it's not running on Node, but on Enonic XP which is currently using Java 8 and the Nashorn js engine within Java.

So if I'm not using flow, and don't need types I have to reimplement my own cheerioEvaluator.

I guess it's easier to simply add @babel/preset-flow then.

I still think the import code is wrong (even in a flow context), but I might be wrong cause I have absolutely no experience with flow.

gajus commented 6 years ago

So if I'm not using flow, and don't need types I have to reimplement my own cheerioEvaluator.

No... Average user does not need to think/ know/ care about Flow.

The code should work out of the box.

If you want to use Flow, then you need to add babel plugins that strip Flow types.

ComLock commented 6 years ago

Pushed https://github.com/ComLock/enonic-xp-app-crawler/blob/master/src/main/resources/main.es#L31

ComLock commented 6 years ago

It works out of the box when I do not explicitly define an evaluator. I was just trying it out in case it affected performance. I felt like 700ms to scrape a string from www.example.com was slow. Rescraping the same string is like 200ms so it might only be the first scrape.

If I'm going to crawl and scrape say 100K documents, then performance might become an issue.

ComLock commented 6 years ago

Luv your work btw.

gajus commented 6 years ago
import {cheerioEvaluator} from 'surgeon/src/evaluators';

This is wrong.

  1. You are referencing the source files, not the files designed for consumption. The only reason source files are included is to enable sourcemap resolution.
  2. You should never (unless stated otherwise in the documentation) deep link project files. There is no guarantee that the path will not change in the next release.

The method that you are looking for is exported from the entry file.

https://github.com/gajus/surgeon/blob/master/src/index.js

The correct way is therefore:

import {cheerioEvaluator} from 'surgeon';
ComLock commented 6 years ago

Thanks.

I'm not too worried about deep linking as that would only give me a webpack build error, but yeah you're right. If I'm deep linking I need to have the same build setup too. So it's a bad idea.

I guess I've been enjoying too many freedoms working in a non Node environment :)