Giter Club home page Giter Club logo

Comments (52)

kentcdodds avatar kentcdodds commented on August 15, 2024 2

@edm00se has discovered that this is resolved when setting {shell: true} in the options, which we do in next. This means that as soon as this lands, this issue will be fixed!

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024 2

There is not I'm afraid. I'm not sure how to solve the problem honestly. Suggestions and pull requests are welcome.

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024 1

By the way, reproducing this on Linux-based environments is also possible by using something like %npm_package_name%. This is also replaced by $npm_package_name, but not evaluated.

from cross-env.

hgwood avatar hgwood commented on August 15, 2024 1

@edm00se Right. It does work now. I probably forgot to build or something. Sorry.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024 1

Could you try this out with cross-env@beta in your project and let us know if it fixes things? I'm most uncertain about this issue, so I want to make sure it's resolved before officially releasing beta. Thanks!

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024 1

Ok, so, to make sure I understand, you're saying that if you use cross-env-shell it works, but it doesn't work with cross-env?

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024 1

Yeah, I'm pretty sure this is part of why cross-env-shell was created in the first place. Please use that. Thanks!

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

This is definitely a bug. Any chance you could create a repository that reproduces this issue? Thanks!

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

Actually a single package.json file with the following content would make it:

{
  "name": "my-nice-app",
  "version": "1.0.0",
  "scripts": {
    "echo_name": "cross-env echo $npm_package_name"
  },
  "devDependencies": {
    "cross-env": "^3.2.4"
  }
}

and then run npm run echo_name in Windows cmd, Cygwin or VSCode bash.

I would expect this to display my-nice-app instead of %npm_package_name%

Additional info:

$ node -v
v6.9.1
$ npm -v
4.2.0

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Sorry, I was unable to reproduce. This works just fine:

~/Desktop
πŸ”  $ git clone https://github.com/kentcdodds/cross-env-npm-env-vars.git
Cloning into 'cross-env-npm-env-vars'...
remote: Counting objects: 88, done.
remote: Compressing objects: 100% (69/69), done.
remote: Total 88 (delta 9), reused 88 (delta 9), pack-reused 0
Unpacking objects: 100% (88/88), done.
~/Desktop
πŸ”  $ cd cross-env-npm-env-vars/
/Users/kdodds/Desktop/cross-env-npm-env-vars
~/Desktop/cross-env-npm-env-vars (master)
πŸ”  $ npm install
[email protected] /Users/kdodds/Desktop/cross-env-npm-env-vars
└─┬ [email protected] 
  β”œβ”€β”¬ [email protected] 
  β”‚ β”œβ”€β”¬ [email protected] 
  β”‚ β”‚ β”œβ”€β”€ [email protected] 
  β”‚ β”‚ └── [email protected] 
  β”‚ β”œβ”€β”¬ [email protected] 
  β”‚ β”‚ └── [email protected] 
  β”‚ └─┬ [email protected] 
  β”‚   └── [email protected] 
  └── [email protected] 

~/Desktop/cross-env-npm-env-vars (master)
πŸ”  $ npm run echo_name

> [email protected] echo_name /Users/kdodds/Desktop/cross-env-npm-env-vars
> cross-env echo $npm_package_name

cross-env-npm-env-vars
~/Desktop/cross-env-npm-env-vars (master)
πŸ”  $ 

Try it yourself:

git clone https://github.com/kentcdodds/cross-env-npm-env-vars.git
cd cross-env-npm-env-vars/
npm install
npm run echo_name

Note: This is why I ask for a repository that reproduces the issue. Because more often than not you'll find out that it's not related to cross-env at all, but is something else entirely. Good luck figuring out what it is! :)

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

Quick close... Your example (no more than just a package.json file) is failing on my side

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Your example (no more than just a package.json file) is failing on my side

Ok, thanks for letting me know. Unfortunately that doesn't really help us know how to solve the issue. Could you please dig a little deeper and get more information (or try to fix the issue)?

from cross-env.

wolfgang42 avatar wolfgang42 commented on August 15, 2024

I have encountered this same issue just now. @kentcdodds, I can confirm that your reproduction is also failing for me. I am running Windows 10, NPM 3.10.8, and NodeJS 6.8.1. I can reproduce the problem from both Git Bash and the Windows CMD.exe.

$ git clone https://github.com/kentcdodds/cross-env-npm-env-vars
Cloning into 'cross-env-npm-env-vars'...
remote: Counting objects: 88, done.
remote: Compressing objects: 100% (69/69), done.
remote: Total 88 (delta 9), reused 88 (delta 9), pack-reused 0
Unpacking objects: 100% (88/88), done.
Checking connectivity... done.
$ cd cross-env-npm-env-vars/
$ npm i
[email protected] C:\Users\wfaust\Documents\Development\cross-env-npm-env-vars
`-- [email protected]
  +-- [email protected]
  | +-- [email protected]
  | | +-- [email protected]
  | | `-- [email protected]
  | +-- [email protected]
  | | `-- [email protected]
  | `-- [email protected]
  |   `-- [email protected]
  `-- [email protected]
$ npm run echo_name

> [email protected] echo_name C:\Users\wfaust\Documents\Development\cross-env-npm-env-vars
> cross-env echo $npm_package_name

%npm_package_name%

I will investigate further and see if I can determine why this is happening.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Thanks for the added confirmation @wolfgang42. Reopening.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

@cvillerm, you're also on Windows correct? This appears to be a windows-only issue.

from cross-env.

wolfgang42 avatar wolfgang42 commented on August 15, 2024

Actually, it seems to be any environment variable, not just ones defined outside of cross-env. I see the same problem with this script:

"scripts": {
  "echo_name": "cross-env testvar=foo echo $testvar"
}

which results in %testvar% instead of the expected foo.

This looks like it might be a bug in the cross-spawn package, perhaps? The variable syntax is being converted by commandConvert(), but I'm not sure at what point variable substitution is supposed to be happening.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Happens here. Unfortunately it's not super simple, but the basic idea is that cross-env simply removes all instances of setting environment variables and adds them to envVars which is then used in the spawn call. In the case of npm_package_name however, that variable should be pre-loaded into process.env when cross-spawn starts, so it should already be included here.

Could you verify that it is included at that point and if it's not try to figure out why it's not in process.env when cross-env starts out. Because npm run should be setting that.

from cross-env.

wolfgang42 avatar wolfgang42 commented on August 15, 2024

Yes, @cvillerm is on Windows.

This results in the following on Windows (with Windows cmd, CygWin bash or VSCode bash)...

I have verified that process.env.npm_package_name is set, and that the correct value is in the env variable by index.js:11.

from cross-env.

hgwood avatar hgwood commented on August 15, 2024

Actually...

> cross-env A=1 echo $A
%A%
> cross-env A=1 node -e "console.log(process.env.A)"
1
> echo echo %A% > echo.bat
> cross-env A=1 echo.bat
1

I'm not sure echo can be trusted here.

from cross-env.

wolfgang42 avatar wolfgang42 commented on August 15, 2024

@hgwood You're looking at the wrong thingβ€”the environment variable is set, but not substituted in the arguments. Instead of process.env you should be looking at process.argv. Similarly, in your echo.bat it's cmd.exe doing the argument substitution from the environment variables.

from cross-env.

hgwood avatar hgwood commented on August 15, 2024

cross-env does not substitute variables for their value, it only converts the syntax from $a to %a%.

from cross-env.

wolfgang42 avatar wolfgang42 commented on August 15, 2024

from cross-env.

hgwood avatar hgwood commented on August 15, 2024

You are right. It does not make much sense. I have only been a contributor for a few days so I have looked at the history. cross-env was made to handle setting env vars, not reading them. Some support for reading them was introduced in #32, but it only works in very specific scenarios (I don't know which).

I have recently been working on improving this. See #86, #89, #90. I would assume #89 fixes the problem we are discussing, but I have just run a few tests and it looks like it does not. The first solution discussed in #90 would though.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

I've update the reproduction repo to use node -e instead of echo which is unreliable as noted :)

So, we all need to remember that cross-env was never intended to replace environment variable references in the command with the value. Neither is cross-var for that matter. Their job is to simply change the script to use the syntax compatible for the platform on which they run. Then spawn that script. From there, the OS takes care of things.

So now if you run this you should get the correct output:

git clone https://github.com/kentcdodds/cross-env-npm-env-vars.git
cd cross-env-npm-env-vars/
npm install
npm run echo_name

cross-env is working as designed and supporting the use cases you should use it for.

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

Thanks both for looking deeper into that. Obviously, this wasn't the result I was looking for and this confirms that this seems to be an acceptable limitation of this command for environment variables defined "outside of" cross-env in Windows, as this issue was about.

Although cross-var is much less used than cross-env, it worked fine with such npm package-based variable. I may have to look at that direction.

Thanks again

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Hmmm... I'm pretty confident that even without cross-env you would see the same result. This is just how things work unless I'm missing something...

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Ah, after looking more closely at the cross-var implementation it appears that it's actually replacing the string values with the actual values rather than spawning the command with the values set in process.env. I guess it's just a difference in perspective. Do you want to execute a script that would behave exactly the same as echo %npm_package_name% or do you want to execute a script that replaces the value first? Ultimately, you should get the exact same result in any real-world scenario.

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

The way I would like to use environment variables would be to use them as part of an npm script definition, not only within the executed script, for instance to create a docker image with a tag based on the name as defined in the package.json file, e.g.:

"build_docker": "docker build -t $npm_package_name ."

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Yes, that should work with cross-env. Have you tried it?

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

It doesn't work (in Windows) as cross-env docker build -t $npm_package_name . as it doesn't with the simpler example I provided in this issue with cross-env echo $npm_package_name

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

At https://github.com/kentcdodds/cross-env/blob/master/src/command.js#L18, what about replacing:

 command = isWin ? `%${match[1]}%` : `$${match[1]}`

by

 command = process.env[match[1]];

This would work for environment variables set before cross-env is called. Being able to replace locally set variables would require using envVars instead

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

And by the way, using parent/child scripts can be a workaround to make it work without an update:

  "scripts": {
    "echo_name": "cross-env echo $npm_package_name",
    "echo_name_parent": "cross-env npm run echo_name"
  },

npm run echo_name_parent will then work as expected. The same is true for more complex commands.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Yeah, this is pretty odd. I definitely want to solve this issue. However I don't want to go with your proposed solution due to the unintended side-effects that could cause (there's no guarantee that replacing the variable with its value will even result in valid syntax). But let's figure out how to solve this issue.

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

Yes, I agree that this needs to be fixed in a cleaner way ... and without breaking current unit tests

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

Great ! Thanks for following this up.

from cross-env.

hgwood avatar hgwood commented on August 15, 2024

@cvillerm could you checkout next and confirm it is fixed there? I have done a few test and it looks like it is not fixed, but I might be doing something wrong.

from cross-env.

edm00se avatar edm00se commented on August 15, 2024

@hgwood This looks to be working from my cloned copy of the next branch. Is there a better test to run?

crossenv-next

from cross-env.

edm00se avatar edm00se commented on August 15, 2024

I forgot to build first as well. πŸ˜„

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Thank you for verifying! I'm so glad this will be fixed soon!

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

Sorry if I didn't answer before, but it takes me ages to run npm install on a clone of cross-env and trying to run it locally doesn't work for me (src/bin/cross-env.js ...). I am not sure what's wrong with it.

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Sorry, it's not the most straightforward thing to do to be honest. But I've updated the reproduction repo and it should work. Just do this (again):

git clone https://github.com/kentcdodds/cross-env-npm-env-vars.git
cd cross-env-npm-env-vars/
npm install
npm run echo_name

And hopefully it will work!

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

I was referring to cloning the cross-env repository, checking out its "next" branch and running npm install to install its own dependencies to test it locally (without referring to it from another test repo).

I anyway also have a problem installing your cross-env-npm-env-vars example, as referring to cross-env as kentcdodds/cross-env#next as devDependency doesn't seem to work in my environment, while referring to the published version works fine. I am behind a proxy, but my environment is normally configured fine to work behind a proxy and I am used to it. Adding that I am developing in Windows is likely adding other constraints.

I am giving up but I am trusting that this will work when the next version is published.

from cross-env.

cvillerm avatar cvillerm commented on August 15, 2024

@kentcdodds , I tested this beta version on Windows (git bash and WIndows cmd) with your test project (cross-env-npm-env-vars) and with one of my projects and this worked fine.

$ npm run echo_name

> [email protected] echo_name D:\Tmp\cross-env-npm-env-vars
> node node_modules/cross-env/dist/bin/cross-env.js echo $npm_package_name

cross-env-npm-env-vars

Thanks!

from cross-env.

paztis avatar paztis commented on August 15, 2024

It seems your version 5.0.1 is still published, with a number upper that beta 5.0.0-beta.0.
Is it possible to publish your fix on 5.0.1-beta.0 ? or to publish it without beta if it works correctly ?

Thanks

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Hmmm... 5.0.1 should have the fix. That's the latest version

from cross-env.

paztis avatar paztis commented on August 15, 2024

from cross-env.

kentcdodds avatar kentcdodds commented on August 15, 2024

Ok. Thanks for bringing it up. Could you dig a little further to see where the problem is and suggest a solution to fix it?

from cross-env.

paztis avatar paztis commented on August 15, 2024

In fact it only works if you define use shell option, because command in exec by node spawn and not a shell.
I've test the command

> cross-env echo $npm_package_name $npm_package_version
%npm_package_name% %npm_package_version%

I see in the code you send it to cross-swpan, that just do a child_process.spawn. And here, if your directly test cross-spwan like this:

const { spawn } = require('cross-spawn');
var child = spawn('echo', ['%PATH%'], { stdio: 'inherit' });

it returns only %PATH% without translation
and same for

const { spawn } = require('child_process');
spawn('echo', ['%PATH%']).on('error', function( err ){ throw err }

The reason it works when we do on a linux

> cross-env echo $npm_package_name $npm_package_version
PACKAGE-NAME 1.0.0

or on window

> cross-env echo %npm_package_name% %npm_package_version%
PACKAGE-NAME 1.0.0

is because the OS replace the value directly becore accession to your function. You can see it by logging commandArgs var inside index.js and you will see the value are already transformed.

For me the only way to resolve it is commandConvert function to replace $xxx by its value if available instead of %xxx% in windows case.
In this case you may need also to pass the getEnvVars as parameter to see the newly added env vars, because they willl be available only new spawn.

from cross-env.

paztis avatar paztis commented on August 15, 2024

from cross-env.

hgwood avatar hgwood commented on August 15, 2024

@paztis Problem solved then?

from cross-env.

hesxenon avatar hesxenon commented on August 15, 2024

hey, sorry to crash the party, but there's still a problem with using cross-env-shell, namely that slashes seem to always be replaced with backslashes on windows.

E.g.: cross-env-shell "echo /" outputs \.

Is there a way to tell cross-env-shell not to replace slashes or to escape certain slashes?

Using [email protected] on Windows 10 with node 8.4.0 and npm 5.3.0

from cross-env.

alexhisen avatar alexhisen commented on August 15, 2024

Note that cross-env-shell and actual substitution of variable only works with the $var syntax, not the %var% syntax, which only gets substituted on Windows.

from cross-env.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.