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

Test failed on node version 10 and 11 #29

Closed
arstgit opened this issue Nov 20, 2018 · 6 comments
Closed

Test failed on node version 10 and 11 #29

arstgit opened this issue Nov 20, 2018 · 6 comments
Assignees

Comments

@arstgit
Copy link

arstgit commented Nov 20, 2018

Looks like error event trigger has changed on new version of node .

@arstgit arstgit changed the title Test failed with node version 10 and 11 Test failed on node version 10 and 11 Nov 20, 2018
@dougwilson
Copy link
Contributor

Thanks for the report, I will take a look!

@dougwilson dougwilson self-assigned this Nov 20, 2018
@dougwilson
Copy link
Contributor

This is due to change of behavior in Node.js 10 that seems unsolvable without changes to Node.js core to restore insight into parser errors.

I filed issues nodejs/node#24586 and nodejs/node#24585

There isn't anything I can really do to fix, unfortunately.

@dougwilson
Copy link
Contributor

Per Node.js core: this module will never work on Node.js 10+

@PixnBits
Copy link

PixnBits commented Mar 18, 2019

That was an adventure ☹️
per nodejs/node#24586 (comment) and expressjs/express#3813 (comment) has this been resolved or is it still a bug?
BTW, thank you for fighting for the users

@dougwilson
Copy link
Contributor

Hi @PixnBits sorry about that 😅 So on Node.js 10 (and 11) this module is fixed … but not fixed. So what I mean by that is the test suite for this module still fails on Node.js 9, but on 10 and 11 the test suite does pass, though there are a bunch of edge cases in this module where it doesn't properly detect the finished state. I have been working on a complete re-write of this module to address theses issue and get it working across all the Node.js versions needed for Express.js 4.x in order to get it included in the next Express.js 4.x release.

@PixnBits
Copy link

Not a problem, thanks so much!

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

No branches or pull requests

3 participants