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

fix(js): node executor address already in use #14023

Merged
merged 1 commit into from Dec 27, 2022
Merged

fix(js): node executor address already in use #14023

merged 1 commit into from Dec 27, 2022

Conversation

sahilpurav
Copy link
Contributor

@sahilpurav sahilpurav commented Dec 26, 2022

Due to this #13813 PR, frameworks like express and nest are throwing the EADDRINUSE error after file changes.
The issue is caused by using array as key in new Map(key, value) function. I corrected it by wrapping JSON.strigify function around Map key.

BREAKING CHANGE:
NA

ISSUES CLOSED: #14017

Current Behavior

Wherever we have @nrwl/js:node executor (express, nest etc.) after each file changes, we are getting EADDRINUSE error.

Expected Behavior

Every file changes should be seamlessly watched by @nrwl/js:node executor and changes should be instantly available while serving the application locally.

Related Issue(s)

#14024
#14023

…rning undefined. issue#14017

There was a breaking change done on @nrwl/js package through #13813
PR, this has caused an issue#14017 on the frameworks like express, nest.js and wherever
@nrwl/js:node is used as executor.
Once the fresh installation of express or nest.js is done
through nx, after every file changes, it gives EADDRINUSE error.
The issue is caused by using
\"array\" as key in \"new Map(key, value)\" function. I corrected it by using JSON.stringify function.

BREAKING CHANGE:
NA

ISSUES CLOSED: #14017
@vercel
Copy link

vercel bot commented Dec 26, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
nx-dev ⬜️ Ignored (Inspect) Dec 26, 2022 at 7:22PM (UTC)

@sahilpurav sahilpurav changed the title fix(js): in new map(key, value), key cannot be an array as it is retu… fix(js): node executor address already in use Dec 26, 2022
@sahilpurav sahilpurav changed the title fix(js): node executor address already in use fix(node): node executor address already in use Dec 26, 2022
@LonguCodes
Copy link

Would be great if someone can merge this

@DavidRojoM
Copy link

Good catch!

@sahilpurav sahilpurav changed the title fix(node): node executor address already in use fix(js): node executor address already in use Dec 27, 2022
@mrfy
Copy link

mrfy commented Dec 27, 2022

Please merge!

@sahilpurav
Copy link
Contributor Author

sahilpurav commented Dec 27, 2022

Please merge!

💯. Waiting for someone to approve. I'm equally eager to have this fix as it is basic requirement to watch the express and nest files. In the meanwhile, build your application on nx@15.3.3

@AgentEnder Help please :)

@AgentEnder AgentEnder merged commit 4c5a139 into nrwl:master Dec 27, 2022
@firxworx
Copy link

@AgentEnder thanks for your help! Seeing as you're at Nx + sort of related, something I was thinking about: since adopting nx for most of my projects these sorts of issues do seem to crop up semi-frequently and overall they can undermine productivity which is against the whole point of Nx as I understand it.

For example

  • Old projects can behave differently than new projects (even with similar stack + apps + etc) in quirky ways
  • Running nx migrate seems OK at first and everything passes but then some new "fun" troubleshooting cases pop up later somewhere in the dev workflow.

Thought/Suggestion

I have no idea what the testing approach is like on your side, so please forgive me if there's anything like this or even better in place and this is just a hard thing to test with a lot of moving parts... It has crossed my mind if some sort of "e2e but for the dev + deploy experience" tests were in place they could help catch many of these sorts of issues pre-release.

For example imagine tests involving both barebones projects and more elaborate multi-app multi-lib setups that really flex the full extent of nx, with scenario coverage of totally new projects being created as well as legacy projects getting updated, etc.

It seems to me that this would be an example of something that could be caught because it would impact a scenario like:

"dev with node project makes changes to app running in dev and the app reloads with their changes"

Failing tests for this issue might be, e.g. that there is error output in the console where none is expected (in this case about an address in use), that the dev's change (e.g. "{ hello: 'world' }" -> "{ bonjour: 'monde' }") wasn't reflected in API response as expected, etc.

Anyway, random two cents. Thanks!

@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Live reload: EADDRINUSE: address already in use
7 participants