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

Improper ES6 import statement breaks (so far unnoticed because of correct CJS result) #892

Open
sarimarton opened this issue Jul 26, 2019 · 2 comments

Comments

@sarimarton
Copy link

Code to reproduce the issue:

index.js:

import { run } from '@cycle/run'
import { timeDriver } from '@cycle/time'

function main (sources) {
  return {}
}

const drivers = {
  Time: timeDriver
}

run(main, drivers)

index.html:

<script src="index.js"></script>

package.json:

{
  "main": "index.html",
  "scripts": {
    "start": "parcel index.html"
  },
  "devDependencies": {
    "@babel/core": "^7.5.5",
    "parcel": "^1.12.3"
  },
  "dependencies": {
    "@cycle/run": "^5.2.0",
    "@cycle/time": "^0.19.0"
  }
}

Open http://localhost:1234/ in the browser.

Expected behavior:

Nothing happens, empty screen, no log message.

Actual behavior:

JS error thrown: Uncaught TypeError: requestAnimationFrame is not a function

Versions of packages used:

See above in package.json

I provided a fix at #891

The problem is this 2 lines in @cycle/time/lib/es6/src/time-driver.js:

// WRONG
import * as requestAnimationFrame from 'raf';
import * as now from 'performance-now';

Those 2 modules export functions, not namespace objects. import * always creates a plain namespace object, which can't be invoked. The correct import statement is without the wildcard:

// WORKS OK
import requestAnimationFrame from 'raf';
import now from 'performance-now';

This bug was unnoticed so far, because most bundler setups are pulling the cjs version (@cycle/time/lib/cjs/src/time-driver.js) instead of the es6 one, and both types of import statements produces the same var <f> = require('...') statement in cjs, which works, because it pulls in the module.exports object - in these cases the function itself.

The es6 version is broken however, and with bundlers which use the es6 version, the result bundle will break.

@sarimarton
Copy link
Author

Currenlty a workaround for the problem is to force loading the cjs version:
import { timeDriver } from '@cycle/time/lib/cjs'

@sarimarton
Copy link
Author

Bump.

Hello guys, this is actually an easy fix, and a pretty basic bug... Can someone please take a look?

Here is an online repro: https://codesandbox.io/s/cycletime-improper-import-statement-kgd02

I don't know about the CI builds and tests (I tried, but they were not up-to-date, I see there's some work on it nowadays), but I know that the fix doesn't change the cjs result, so no tests should be impacted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant