Skip to content

Adds Support for Yarn Install If Supported #2537

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

Merged
merged 2 commits into from
Apr 15, 2017
Merged

Adds Support for Yarn Install If Supported #2537

merged 2 commits into from
Apr 15, 2017

Conversation

benmarten
Copy link
Contributor

@benmarten benmarten commented Apr 7, 2017

checks if yarn is available and runs yarn install on setup. falls back to npm if yarn is not
installed.

@benmarten benmarten mentioned this pull request Apr 8, 2017
gulpfile.js Outdated
@@ -148,7 +148,9 @@ gulp.task('installFixtures', function() {
}, 1 * 1000);
shell.cd('test/fixtures');

execAsync('npm install --quiet', {cwd: '../fixtures'}).then(() => {
execAsync('type yarn &> /dev/null | yarn install || npm install --quiet', {
Copy link
Member

Choose a reason for hiding this comment

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

we should be checking if the user is on Windows, and using the proper Windows commands if so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not on windows and don't have access to it atm.
I found this: https://superuser.com/questions/175466/determine-if-command-is-recognized-in-a-batch-file
Hence we could use: WHERE yarn >nul 2>nul; IF %ERRORLEVEL% EQU 0 yarn install ELSE npm install --quiet;
Can you or sb. confirm that this works please?

Copy link
Member

Choose a reason for hiding this comment

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

This worked for me:

yarn --version >nul 2>&1 && ( yarn install ) || ( npm install --quiet )

We should be able to use process.platform to figure out what the user is on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Tested regular install on macOS.
Tried running: gulp installFixtures but that doesn't work.

Can you test on Windows too please?

@Awk34
Copy link
Member

Awk34 commented Apr 14, 2017

The Windows check fails, since indexOf('win') on 'win32' is 0 (falsey). Just check process.platform === 'win32' instead

@benmarten
Copy link
Contributor Author

Gotta, I thought there might be win64, changed it to win32 now though ;)

@Awk34
Copy link
Member

Awk34 commented Apr 14, 2017

Yeah, that would make sense, but the process docs only has win32. There's a different arch property for 32 vs 64 bit.

Anyhow, your code will still not work on Windows, since process.platform.indexOf('win32') on Windows will result in 0, which is falsey and will not enter the if block.

You can either use process.platform.indexOf('win32') >= 0, or process.platform === 'win32'

checks if yarn is available and runs yarn install on setup. falls back to npm if yarn is not
installed.
@benmarten
Copy link
Contributor Author

updated.

@Awk34 Awk34 merged commit faf4399 into angular-fullstack:master Apr 15, 2017
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.

2 participants