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

NodeJS 10.x compatibility #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

NodeJS 10.x compatibility #2

wants to merge 1 commit into from

Conversation

kaij
Copy link

@kaij kaij commented Jun 12, 2018

Unfortunately, NodeJS >=9.11.0 introduces the socket event "ready", which is also used as a custom event in the code. Renamed this event (plus deprecated buffer constructors) to make lib work with NodeJS 10.x

@mfreeman-xtivia
Copy link

Could I please add a +1 to get this PR added to the baseline?

I spent hours and hours chasing down why this library "broke" after a node upgrade, only to come to the exact same conclusion, i.e. the use of the internal event "ready" here has been co-opted by a system event of the same name.

Would really prefer to avoid having to use a fork for the sake of 2 lines of changes. Thanks

@mfreeman-xtivia
Copy link

Alrighty then, since its crickets here guess I'm off to create my own fork....

@yuchi
Copy link

yuchi commented Oct 28, 2018

Just a small note: @robframpton created this package as part of the Liferay Theme SDK, but has since moved from the company.

Probably he should move the ownership of the repo and npm module to Liferay.

I'll try to contact some one.

@izaera
Copy link
Collaborator

izaera commented Oct 29, 2018

Hey guys: I’m planning to move all that code to a library of its own or to liferay-npm-build-tools because it may also be used from the bundler in the future, but we haven’t had time yet.

We've been busy preparing the events (DEVCON, LSNA, Spain & Italy Sympo, ...) and @jbalsas is not available for the next two weeks 😓 . I guess we can have a look at it then...

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

Successfully merging this pull request may close these issues.

4 participants