Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using esm (ES6 modules) causes failure. #3382

Closed
Rob-pw opened this issue Oct 10, 2018 · 20 comments
Closed

Using esm (ES6 modules) causes failure. #3382

Rob-pw opened this issue Oct 10, 2018 · 20 comments
Labels
chromium Issues with Puppeteer-Chromium

Comments

@Rob-pw
Copy link

Rob-pw commented Oct 10, 2018

Steps to reproduce

Tell us about your environment:

  • Puppeteer version: 6.4.1
  • Platform / OS version: Linux latitude 4.15.0-36-generic Bikeshed the API #39-Ubuntu SMP Mon Sep 24 16:19:09 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • URLs (if applicable):
  • Node.js version: 10.10.0

What steps will reproduce the problem?

Please include code that reproduces the issue.

  1. copy example code into a new file example.js
  2. replace const puppeteer = require('puppeteer') with ìmport puppeteer from 'puppeteer'
  3. run with node -r esm example.js

What is the expected result?
Empty prompt, example.png in script folder.

What happens instead?

node -r esm demo.js 
(node:11730) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'on' of undefined
    at Function.addEventListener (../node_modules/puppeteer/lib/helper.js:165:13)
    at new NavigatorWatcher (../node_modules/puppeteer/lib/FrameManager.js:1146:14)
    at FrameManager.navigateFrame (../node_modules/puppeteer/lib/FrameManager.js:75:21)
    at Frame.goto (../node_modules/puppeteer/lib/FrameManager.js:404:37)
    at Page.goto (../node_modules/puppeteer/lib/Page.js:579:49)
    at require (../demo.js:6:14)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:11730) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
@JoelEinbinder
Copy link
Collaborator

JoelEinbinder commented Oct 12, 2018

I can't reproduce this. I did renamed screenshot.js into screenshot.mjs. I replaced the require with an import. I ran it with node --experimental-modules screenshot.mjs using node v10.10.0. It ran successfully.

@ydnar
Copy link

ydnar commented Oct 12, 2018

I can reproduce this using Puppeteer 1.9.0 with ava using ESM import statements.

@n8io
Copy link

n8io commented Oct 15, 2018

Same.

Puppeteer version: v1.9.0
Platform / OS version: MacOS High Sierra v10.13.6
Node.js version: 10.12.0
ESM package version: 3.0.84
Start script: node -r esm ./example.js

I had to revert back to Puppeteer v1.8.0 from v1.9.0 to get back to a working state.

@xukaifengit
Copy link

I also have this situation.When the high concurrent operation, it will report this exception from time to time, so annoying

@AVGP
Copy link
Contributor

AVGP commented Oct 18, 2018

I'm wondering why you use the esm module instead of the experimental ESM module support in node 10.12?

This works:

node --experimental-modules examples/screenshot.mjs

This does not:

npm i esm
node -r esm examples/screenshot.js

@ydnar
Copy link

ydnar commented Oct 18, 2018

I’m using ava, which controls Node and compilation. This is the relevant issue:

avajs/ava#1810

The error we’re seeing:

Rejected promise returned by test. Reason:

  TypeError {
    message: 'Cannot read property \'on\' of undefined',
  }

  Function.addEventListener (node_modules/puppeteer/lib/helper.js:165:13)
  new NavigatorWatcher (node_modules/puppeteer/lib/FrameManager.js:1146:14)
  FrameManager.navigateFrame (node_modules/puppeteer/lib/FrameManager.js:75:21)
  Frame.goto (node_modules/puppeteer/lib/FrameManager.js:404:37)
  Page.goto (node_modules/puppeteer/lib/Page.js:579:49)
  goto (test/helpers/puppeteer-page.js:17:24)
  visit (test/integration/search.test.js:17:9)

@aslushnikov
Copy link
Contributor

@Rob-pw: please use the --experimental-modules. This is the way forward with ESM in node.js

AVA users: there's avajs/ava#1810 that covers ESM support in ava.

@damianobarbati
Copy link

+1 on this, @Rob-pw thanks for spotting it: after bumping to 1.9.0 I really had no idea where this error was coming from.
@AVGP @aslushnikov currently experimental-modules flag is not very much an option, current .mjs and native ESM support in node is very limited.
ESM package is a great bridge in these years of transition.

@jdalton
Copy link

jdalton commented Nov 3, 2018

Hi @aslushnikov!

👋 Node core and Node Module WG member here. There is no set date for when --experimental-modules will be unflagged. There's also no guarantee that what has shipped now is what will be in the future. For example, there are several discussions in the Node Module WG on how to approach implementation. One such discussion is to ship a maximally minimal implementation that would require package-maps for resolving bare specifiers (no longer using the Node module resolution algorithm).

The --experimental-modules flag really does mean experimental and shouldn't be relied on for anything beyond kicking the tires.

I'm open to digging into it from the esm side of things.
Do you have any idea what could have changed between v1.8.0 from v1.9.0?

@damianobarbati
Copy link

@aslushnikov after digging into the code I noted that this does not seem to not be related to ESM but to this line into into Connection.js:
https://github.com/GoogleChrome/puppeteer/blob/d547b9d24a803c0adf923b3a821ae5f43e79bb53/lib/Connection.js#L51
which is called here into FrameManager.js:
https://github.com/GoogleChrome/puppeteer/blob/c237947b324338ece670b4f4e879880265ec6965/lib/FrameManager.js#L1146

  • connection._connection is undefined
  • connection instanceof CDPSession is true
  • loop starts and connection._connection which is undefined is assigned to connection
  • undefined is not an instance of CDPSession, "while" loop stops and undefined is returned
  • helper.addEventListener(Connection.fromSession(client), ..., ...) attempt to attach event on undefined and the error is fired

Replacing the following into https://github.com/GoogleChrome/puppeteer/blob/d547b9d24a803c0adf923b3a821ae5f43e79bb53/lib/Connection.js#L50 solves the issue

while (connection instanceof CDPSession && connection._connection)

But I'm not sure whether that makes "logic" sense.

@aslushnikov
Copy link
Contributor

I'm open to digging into it from the esm side of things.

@jdalton awesome, reopening.

Do you have any idea what could have changed between v1.8.0 from v1.9.0?

Thanks to @damianobarbati input, I've debugged it down to d547b9d. The core issue seems to be that
an instance of Connection class satisfies the instanceof check against CDPSession class.

Here's a small snippet from Connection class (check out the console.log statements):
image

@jdalton any ideas what's going on here?

@aslushnikov aslushnikov reopened this Nov 4, 2018
@jdalton
Copy link

jdalton commented Nov 9, 2018

Thanks @aslushnikov!

I narrowed it down to a faulty Symbol.hasInstance helper on our end (standard-things/esm@111328e).

@aslushnikov
Copy link
Contributor

@jdalton awesome, thank you for your time! Do you plan to release a new version of esm anytime soon that will include the fix?

@jdalton
Copy link

jdalton commented Nov 9, 2018

Yes, likely within a week or so.

brettz9 added a commit to brettz9/canvg that referenced this issue Nov 20, 2018
…ss npx babel-node will overcome)

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
brettz9 added a commit to brettz9/canvg that referenced this issue Nov 21, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
brettz9 added a commit to brettz9/canvg that referenced this issue Nov 21, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
brettz9 added a commit to brettz9/canvg that referenced this issue Nov 22, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
brettz9 added a commit to brettz9/canvg that referenced this issue Nov 22, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
@prateekbh
Copy link

@jdalton is the release done?

@jdalton
Copy link

jdalton commented Nov 27, 2018

Well that week-or-so turned into a bit longer with the US Thanksgiving holiday. I should release before I start more aggressive perf work. I'll ping back this thread when it's released.

brettz9 added a commit to brettz9/canvg that referenced this issue Dec 4, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
@aslushnikov aslushnikov added the chromium Issues with Puppeteer-Chromium label Dec 6, 2018
brettz9 added a commit to brettz9/canvg that referenced this issue Dec 27, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
brettz9 added a commit to brettz9/canvg that referenced this issue Dec 28, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
brettz9 added a commit to brettz9/canvg that referenced this issue Dec 29, 2018
…ss npx babel-node will overcome); For startsWith/endsWith requires: - Breaking change: Require `@babel/polyfill`

- Breaking change (minor): `svg.Property` (2nd arg) (and `svg.ViewPort.ComputeSize` (1st arg)) may only accept string literals (no String objects)
- Enhancement: Add ESM distribution file (and point to it from `package.json` `module`) and Babelify source
- Linting (ESLint): Rename to `.js` to allow commenting
- Linting (ESLint): Apply most of strict `ash-nazg/sauron` linting (consistent semicolons, single quotes; prefer const then let; object destructuring and shorthand; avoid eqeq, quoted properties, and unnecessary parentheses; space before comments; Unicode regexes; parentheses with `new`; avoid shadowing; switch indents; guard for-in; avoid direct use of `hasOwnProperty`; fix JSDoc;
import order based on import type; prefer exponentiation operator, startsWith/endsWith, addEventListener; avoid unsafe regex)
- Refactoring: Make clear that `getBoundingBox` and `elementTransform` of `svg.Element.use` and `svg.Element.tref.getText` return `undefined` in some cases
- Docs: Lint JS in README
- npm: Update nested packages
@aslushnikov
Copy link
Contributor

I don't think we can do anything else on the puppeteer-side for this. Once @jdalton publishes a new version of esm, the issue will be mitigated.

@sergiitk
Copy link
Contributor

sergiitk commented Jan 11, 2019

BTW, I had this issue with puppeteer 1.9.0, had to roll back to 1.8.0 which didn't produce the error. Then accidentally updated it 1.11.0 and it just worked! Here's my setup: https://github.com/sergiitk/pagerbeauty/blob/122aca70976db520487906ac670b98d1177b275c/Dockerfile-test-acceptance

Chromium 71 / Ava 1.0.1 / Puppetter 1.11.0 / Docker / Node 10.15.0-alpine

@nuragic
Copy link

nuragic commented May 16, 2019

👋 Hello @jdalton! We have a script using puppeteer, it was working great with esm@3.2.22, BUT, we just updated to the latest version i.e. esm@3.2.23 (released 8h ago) and well it just broke everything! 😅

We got this error:

static async create(client, target, ignoreHTTPSErrors, defaultViewport, screenshotTaskQueue) {
             ^

SyntaxError: Invalid or unexpected token
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:712:10)

Any idea? Thanks in advance! 🙏

Update: I just found the related issue in the esm repo: standard-things/esm#804

@reesericci
Copy link

Is ESM currently working? @aslushnikov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chromium Issues with Puppeteer-Chromium
Projects
None yet
Development

No branches or pull requests