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

feat: Refactor the ipc module. #7076

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

feat: Refactor the ipc module. #7076

wants to merge 4 commits into from

Conversation

ygj6
Copy link
Member

@ygj6 ygj6 commented Mar 25, 2022

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Underlying tools
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

Other information:

  • The dependencies of @achrinza/node-ipc has security risks.
    • The owner of js-message and easy-stack remains Brandon Nozaki Miller.
    • @achrinza/node-ipc does not accept new features.
  • Rewrite the iPc module to eliminate dependencies.
  • More Robust Custom Communication Protocols.
  • Remove emitevent to reduce performance overhead.

@ygj6
Copy link
Member Author

ygj6 commented Mar 25, 2022

Local is passed. let me see.:sweat_smile:

@haoqunjiang
Copy link
Member

Their versions are fixed, npm does not allow overwriting existing versions, so no risk.

  • @achrinza/node-ipc does not accept new features.

We don't need new features, either. As long as there are no vulnerabilities we are fine.

@RedNuttyGuy
Copy link

Hi,

Is there an estimate on when this change will be released?

@achrinza/node-ipc recently updated fixing an issue with non-lts versions of node and yarn caused by the engines.node config.

If it's going to be a while away I can create a PR to update the dependency.

Copy link
Member

@haoqunjiang haoqunjiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern with this PR is that it makes CLI UI not able to communicate with projects running older versions of @vue/cli-service via IPC.

Is there anyway to keep that compatibility?

@ygj6
Copy link
Member Author

ygj6 commented Apr 6, 2022

To be compatible with older versions of @vue/cli-service, we can use the same pipe path as node-ipc. I'll verify it further.

@ygj6
Copy link
Member Author

ygj6 commented Apr 9, 2022

Is there anyway to keep that compatibility?

It can be compatible with older versions after using the same ipc path and message format. I have tested it on the dashboard page.

@ygj6 ygj6 requested a review from haoqunjiang April 9, 2022 06:22
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.

3 participants