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

ESM modules breaking tests #517

Closed
djMax opened this issue Mar 6, 2019 · 13 comments
Closed

ESM modules breaking tests #517

djMax opened this issue Mar 6, 2019 · 13 comments

Comments

@djMax
Copy link

djMax commented Mar 6, 2019

At the least, I think this is a semver violation, but not sure the root cause. As of tap 12.4.0 our tests work fine, with 12.5, they all fail with this:

> tap --node-arg=-r --node-arg=@babel/register "tests/test_startup.js"

tests/test_startup.js ................................. 1/2 5s
  start app for test_startup
  not ok Unexpected token import
    stack: |
      SyntaxError: Unexpected token import
          Object._compile (node_modules/pirates/lib/index.js:99:24)
          Object.newLoader [as .js] (node_modules/pirates/lib/index.js:104:7)
      From previous event:
          GbService.configure (node_modules/@gasbuddy/service/src/Service.js:108:26)
          <anonymous>
    type: SyntaxError
    tapCaught: returnedPromiseRejection
    test: start app for test_startup

total ................................................. 1/2

I'm not sure the logic behind adding esm to tap, but it breaks babel in some way.

@isaacs
Copy link
Member

isaacs commented Mar 6, 2019

@jdalton, is this related to the stuff that you were just working on to get it playing more nicely with NYC?

@jdalton
Copy link

jdalton commented Mar 6, 2019

How does tap order the --node-arg additions. Is esm loaded after or before?

@isaacs
Copy link
Member

isaacs commented Mar 7, 2019

esm is loaded first, then whatever other node-arg arguments get appended to that.

@jdalton
Copy link

jdalton commented Mar 7, 2019

@isaacs Perfect! (esm has scenario tests with @babel/register)

@djMax Could you produce a small repro. That would help me diagnose the issue and get a fix out.

isaacs added a commit that referenced this issue Mar 7, 2019
@isaacs
Copy link
Member

isaacs commented Mar 7, 2019

In the meantime, update to tap v12.6, and add --no-esm if you prefer.

@isaacs isaacs closed this as completed Mar 7, 2019
@jdalton
Copy link

jdalton commented Mar 7, 2019

@djMax I tried to simulate a repro and could not reproduce the issue on my end. If you manage to create a repro please feel free to post back here or at standard-things/esm/issues.

@djMax
Copy link
Author

djMax commented Mar 7, 2019

This module: https://github.com/gas-buddy/service if you install tap@12.5.0, will fail

@delvedor
Copy link

delvedor commented Mar 7, 2019

I had a similar problem as well, and this is my log:

.../node_modules/tap/lib/stack.js:1
TypeError: Cannot read property 'value' of undefined
    at Object.Module._extensions..js (module.js:586:10)

It happens only in Node v6. Updating to tap v12.6.0 and using --no-esm fixes the issue.

@jdalton
Copy link

jdalton commented Mar 7, 2019

@djMax Thanks, digging in.

Update:

Patched standard-things/esm@8b8bcd1

Update:

esm v3.2.12 is released 🎉

@delvedor If you have a small repro I'll dig into that too 💪

@delvedor
Copy link

delvedor commented Mar 8, 2019

@jdalton thanks! Unfortunately, I haven't. In my project I'm running tap in three different test suites and only one of them is failing. In none of them I'm using esm. So I have no idea what is causing this issue.

@jdalton
Copy link

jdalton commented Mar 8, 2019

What version of Node 6 are you using?

@delvedor
Copy link

delvedor commented Mar 8, 2019

v6.17.0

@jdalton
Copy link

jdalton commented Mar 8, 2019

Thanks @delvedor!

I narrowed down the issue to an engine bug in Node 6 I have to guard against. It seems in Node 6 a generator may not always return an object with .value or .done and may return undefined instead.

Update:

Related to this V8 bug https://bugs.chromium.org/p/v8/issues/detail?id=5322 with the --harmony flag.

Update:

Patch standard-things/esm@23a163a;

Update:

esm v3.2.14 is released 🎉

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

4 participants