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

production flag in Version 3.8.3 does not work for older npm versions #98

Closed
gemafrzen opened this issue Aug 19, 2024 · 2 comments · Fixed by #99
Closed

production flag in Version 3.8.3 does not work for older npm versions #98

gemafrzen opened this issue Aug 19, 2024 · 2 comments · Fixed by #99

Comments

@gemafrzen
Copy link

Hello,

the added code in pull request "Use "--omit=dev" internally on newer npm version (#86)" is not correct and the version check is always false. Thus the production flag is not used.

I think the solution is to change the following function

function getNpmVersion() {
    var version = child_process_1.exec('npm --version');
    return version.stdout.toString();
}

into

function getNpmVersion() {
  var version = child_process_1.execSync('npm --version');
    return version.toString();
}

In addition please re-add the test for older versions.

Best regards,
Erik

@ericcornelissen
Copy link
Contributor

ericcornelissen commented Aug 21, 2024

the added code in pull request "Use "--omit=dev" internally on newer npm version (#86)" is not correct and the version check is always false. Thus the production flag is not used.

I think the solution is to change the following function

function getNpmVersion() {
    var version = child_process_1.exec('npm --version');
    return version.stdout.toString();
}

into

function getNpmVersion() {
  var version = child_process_1.execSync('npm --version');
    return version.toString();
}

That seems plausible (I can't test this right now unfortunately), do you want to create a PR for this change?

In addition please re-add the test for older versions.

Feel free to submit a PR to run on older version, if they pass it should be good to go anyway.

@jeemok
Copy link
Owner

jeemok commented Sep 2, 2024

Released in 3.9.0. Thank you @ericcornelissen for fixing this!

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 a pull request may close this issue.

3 participants