react-datepicker icon indicating copy to clipboard operation
react-datepicker copied to clipboard

fixed scripts on windows environments

Open dpuhlmann opened this issue 1 year ago • 5 comments

Any script from package.json that sets NODE_OPTIONS and/or NODE_ENV will not work in CMD/PowerShell (nor in Git Bash, which on some occasions can help with windows comand line problems). The affected sripts include "build" and "test" which make local development nearly impossible.

dpuhlmann avatar Jul 24 '23 13:07 dpuhlmann

cross-env should not be used any more. It is EOL as per kentcdodds/cross-env#257. What issue are you trying to solve with this PR?

Image of Graham C Graham C

Reviewed with ❤️ by PullRequest

According to the linked issue cross-env is in maintenance mode not EOL, which should still make it perfectly fine to include. Also there seem to be no good alternatives.

dpuhlmann avatar Jul 24 '23 14:07 dpuhlmann

cross-env should not be used any more. It is EOL as per kentcdodds/cross-env#257. What issue are you trying to solve with this PR? Reviewed with ❤️ by PullRequest

According to the linked issue cross-env is in maintenance mode not EOL, which should still make it perfectly fine to include. Also there seem to be no good alternatives.

That post is a little out of date with regards to that which is why I had said that it is now EOL. The repo was marked as read-only in 2021, a year after the post was made, making it EOL and a security risk to include, since any security issues may never be properly disclosed nor fixed.

Image of Graham C Graham C

Okay, i see your point. But how relevant is that for including it as a dev dependency? It will only be used at build time and not distributed with the package. Also what would be the alternative? Just not supporting development on Windows anymore doesn't sound like a good option.

dpuhlmann avatar Jul 24 '23 15:07 dpuhlmann

cross-env should not be used any more. It is EOL as per kentcdodds/cross-env#257. What issue are you trying to solve with this PR? Reviewed with ❤️ by PullRequest

According to the linked issue cross-env is in maintenance mode not EOL, which should still make it perfectly fine to include. Also there seem to be no good alternatives.

That post is a little out of date with regards to that which is why I had said that it is now EOL. The repo was marked as read-only in 2021, a year after the post was made, making it EOL and a security risk to include, since any security issues may never be properly disclosed nor fixed.

Okay, i see your point. But how relevant is that for including it as a dev dependency? It will only be used at build time and not distributed with the package. Also what would be the alternative? Just not supporting development on Windows anymore doesn't sound like a good option.

Security issues are just as relevant for development, if not more so, because the consequences can be worse if takeover of the developer's machine or access to their GitHub account can be achieved. What exactly is broken on Windows today? Is it mitigated by using WSL?

Image of Graham C Graham C

First to what was (is) broken: Any script from package.json that sets NODE_OPTIONS and/or NODE_ENV will not work in CMD/PowerShell (nor in Git Bash, which on some occasions can help with windows comand line problems). The affected sripts include "build" and "test" which make local development nearly impossible.

Regarding WSL as an alternative: This might be an option for some developers (if it works there, which I do not know). For myself this is not a possibility due to me beeing on a company machine with limited permissions, especially regarding installation of software.

For security risks: Cross-env is talking as input scripts from package.json and process variables. For the scripts I have to already trust them when executing them and they go through the normal contribution process of the library. So they are not more dangerous then any other piece of code in this package. Or to put it the other way around: If someone is able to inject malicous code into the library (or package scripts) cross-env is the more minor problem. For the process variables that is even more true: If an attacker is able to manipulate the process environment on my machine I already have an enormous problem, where again the use cross-env is of secondary concern. Or do you see any other realistic attack vectors that would be introduced by the use of cross-env?

dpuhlmann avatar Jul 25 '23 12:07 dpuhlmann

The affected sripts include "build" and "test" which make local development nearly impossible.

Also what would be the alternative? Just not supporting development on Windows anymore doesn't sound like a good option.

I'm not entirely convinced by this argument, since all they are just helpers or ways to execute commands; have you considered some of the alternatives:

  1. adding a new command, i.e. win:build-dev which runs a windows-compatible (pwsh.exe, cmd.exe) for the rare windows developer with no access to WSL?

  2. Providing a docker image so that we can ensure everyone's developer environment is the same?

I'm not strongly against adding cross-env back, I think the security implications are marginal - but I do think different solutions probably work better than pulling in a unmaintained dependency; but as it's a dev dependency I'm not hugely concerned. Will this unblock you to contribute to this repository or are you adding this fix just because it fixes a problem you're aware of?

Zarthus avatar Aug 08 '23 15:08 Zarthus

Also what would be the alternative? Just not supporting development on Windows anymore doesn't sound like a good option.

You can use dotenvx for this in place of cross-env. It has cross-env support via the following and is actively maintained.

dotenvx run --env="KEY=value" -- yourcommand

https://github.com/dotenvx/dotenvx

motdotla avatar Feb 28 '24 00:02 motdotla

Looks like dotenvx should be sufficient.

martijnrusschen avatar Mar 10 '24 19:03 martijnrusschen