-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Implements "yarn node" #5388
Implements "yarn node" #5388
Conversation
This change will increase the build size from 10.46 MB to 10.46 MB, an increase of 4.26 KB (0%)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand the need for this but I trust you 😃
This is essentially equivalent to having a script inside package.json
making use of the $NODE
environment variable we have set, right?
src/cli/index.js
Outdated
if (command === commands.run || command === commands.create) { | ||
// we are using "yarn <script> -abc", "yarn run <script> -abc", or "yarn node -abc", we want -abc to | ||
// be script options, not yarn options | ||
if (command === commands.run || command === commands.create || command === commands.node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it is time to create ARG_PROXY_COMMANDS = new Set(['run', 'create', 'node']);
or add a new field to all commands, proxyArgs: boolean
to determine this behavior.
Yup! The difference being that it would give us more opportunities to do extra things on the environment if we want to (for example we could imagine logging the require calls to analyze which dependencies are not being used, or stuff like this). |
Could this be used to colocate specific node versions within packages? |
I'm not sure I understand, can you be more specific? :D
…On Fri, Feb 23, 2018, 10:00 PM Cole Turner ***@***.***> wrote:
Could this be used to colocate specific node versions within packages?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#5388 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA_Wa2dN7E4ypRYJFX7nvhNzwFBMzFtMks5tXybxgaJpZM4SNmKV>
.
|
Ah sorry what I mean is that I’ve recently seen some modules include the “node” binary as a dependency. Im wondering if this could be used to run that directly.
Cheers!
… On Feb 23, 2018, at 6:34 PM, Maël Nison ***@***.***> wrote:
I'm not sure I understand, can you be more specific? :D
On Fri, Feb 23, 2018, 10:00 PM Cole Turner ***@***.***> wrote:
> Could this be used to colocate specific node versions within packages?
>
> —
> You are receiving this because you modified the open/close state.
>
>
> Reply to this email directly, view it on GitHub
> <#5388 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AA_Wa2dN7E4ypRYJFX7nvhNzwFBMzFtMks5tXybxgaJpZM4SNmKV>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Summary
This PR implements a new command called
yarn node
. It is meant to call Node with the correct environment to run the application. The main difference with running Node directly is that Yarn will automatically "cd" to the right directory.Test plan
I manually ran the following commands and checked their outputs:
I haven't written unit tests for this feature because it is relatively small and self-contained, and because I'm currently designing a new architecture for our tests, which will include tests for this feature. I prefer to spend time on this 🙂