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(docs,examples,templates): make npm-run-all calls work when using Yarn #3595

Merged
merged 4 commits into from
Jul 2, 2022

Conversation

lensbart
Copy link
Contributor

run-p dev:* doesn’t (but probably should) work with Yarn. Thankfully, this can be fixed quite easily by wrapping the argument in quotes.

(As an aside, I find single quotes a bit cleaner than escaped double quotes, but I saw that escaped quotes were already used elsewhere so I stuck to the convention.)

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Jun 28, 2022

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@lensbart lensbart changed the base branch from main to dev June 28, 2022 23:07
@lensbart lensbart changed the title fix(templates): run run-p with Yarn fix(templates): run run-p with Yarn Jun 28, 2022
@lensbart lensbart mentioned this pull request Jun 29, 2022
@MichaelDeBoey MichaelDeBoey changed the title fix(templates): run run-p with Yarn fix(docs,examples,templates): make run run-p work when using Yarn Jun 29, 2022
@MichaelDeBoey MichaelDeBoey changed the base branch from dev to main June 29, 2022 10:10
Copy link
Member

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

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

We already had #2417 to start working on this.
Since it didn't get any response in the last 25 days, I'd rather get this one merged instead.

1 thing I want to do here too though is also update all run-s calls as well.

I also prefer using single quotes tbh, as that makes it a bit more easy on the eyes (and has a lower cognitive load as well, since you don't need to think about where you are in the script & if you have escaped all of the correctly)

@MichaelDeBoey MichaelDeBoey changed the title fix(docs,examples,templates): make run run-p work when using Yarn fix(docs,examples,templates): make npm-run-all calls work when using Yarn Jun 29, 2022
@lensbart
Copy link
Contributor Author

@MichaelDeBoey consider it done!

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

I'm pretty sure single quotes make this not work on windows. I've never heard of needing to quote these npm-run-all usages for Windows support 🤔

@lensbart
Copy link
Contributor Author

lensbart commented Jul 2, 2022

@kentcdodds fixed

@lensbart lensbart requested a review from kentcdodds July 2, 2022 11:02
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thanks!

@kentcdodds kentcdodds merged commit 967a2f1 into remix-run:main Jul 2, 2022
@lensbart lensbart deleted the 🏃 branch July 2, 2022 15:54
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

🤖 Hello there,

We just published version v1.6.4 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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

Successfully merging this pull request may close these issues.

Stopped working with yarn
3 participants