-
Notifications
You must be signed in to change notification settings - Fork 247
Behavior with environment variables defined "outside of" cross-env #91
Comments
This is definitely a bug. Any chance you could create a repository that reproduces this issue? Thanks! |
Actually a single {
"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 I would expect this to display Additional info: $ node -v
v6.9.1
$ npm -v
4.2.0 |
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 |
Quick close... 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)? |
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.
I will investigate further and see if I can determine why this is happening. |
Thanks for the added confirmation @wolfgang42. Reopening. |
@cvillerm, you're also on Windows correct? This appears to be a windows-only issue. |
Actually, it seems to be any environment variable, not just ones defined outside of "scripts": {
"echo_name": "cross-env testvar=foo echo $testvar"
} which results in This looks like it might be a bug in the |
Happens here. Unfortunately it's not super simple, but the basic idea is that Could you verify that it is included at that point and if it's not try to figure out why it's not in |
Yes, @cvillerm is on Windows.
I have verified that |
Actually...
I'm not sure |
@hgwood You're looking at the wrong thing—the environment variable is set, but not substituted in the arguments. Instead of |
cross-env does not substitute variables for their value, it only converts the syntax from |
Is something else supposed to be performing this substitution, then? It
seems a bit silly to do the conversion if nothing uses the converted values.
…--
The views expressed above are exclusively mine, if anyone's.
|
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. |
I've update the reproduction repo to use So, we all need to remember that 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
|
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 Thanks again |
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... |
Ah, after looking more closely at the |
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 "build_docker": "docker build -t $npm_package_name ." |
Yes, that should work with |
It doesn't work (in Windows) as |
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 |
Yes, I agree that this needs to be fixed in a cleaner way ... and without breaking current unit tests |
By the way, reproducing this on Linux-based environments is also possible by using something like |
@edm00se has discovered that this is resolved when setting |
Great ! Thanks for following this up. |
@cvillerm could you checkout |
@hgwood This looks to be working from my cloned copy of the |
@edm00se Right. It does work now. I probably forgot to build or something. Sorry. |
I forgot to build first as well. 😄 |
Thank you for verifying! I'm so glad this will be fixed soon! |
Sorry if I didn't answer before, but it takes me ages to run |
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! |
I was referring to cloning the cross-env repository, checking out its "next" branch and running I anyway also have a problem installing your cross-env-npm-env-vars example, as referring to cross-env as I am giving up but I am trusting that this will work when the |
Could you try this out with |
@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! |
It seems your version 5.0.1 is still published, with a number upper that beta 5.0.0-beta.0. Thanks |
Hmmm... 5.0.1 should have the fix. That's the latest version |
Problem is still present on my side
Le 12 juil. 2017 16:32, "Kent C. Dodds" <[email protected]> a écrit :
Hmmm... 5.0.1 should have the fix. That's the latest version
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#91 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFCmMbWW9WOng0cW7G05LmznZNWfvCxLks5sNNjWgaJpZM4MibX2>
.
|
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? |
In fact it only works if you define use shell option, because command in exec by node spawn and not a shell.
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:
it returns only %PATH% without translation
The reason it works when we do on a linux
or on window
is because the OS replace the value directly becore accession to your function. You can see it by logging For me the only way to resolve it is |
Ok, so, to make sure I understand, you're saying that if you use |
Yes that's it
Le 13 juil. 2017 01:09, "Kent C. Dodds" <[email protected]> a écrit :
… 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?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#91 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFCmMZf_3eH2UqAfiPRS3afo9UrpHo1aks5sNVI_gaJpZM4MibX2>
.
|
@paztis Problem solved then? |
Yeah, I'm pretty sure this is part of why |
hey, sorry to crash the party, but there's still a problem with using E.g.: 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 |
There is not I'm afraid. I'm not sure how to solve the problem honestly. Suggestions and pull requests are welcome. |
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. |
While trying to find a solution to write npm scripts command lines using npm variables (e.g.
npm_package_name
), I found thatcross-env
doesn't see to recognize them, as if only variables defined usingcross-env
would be recognized.For instance, with a
package.json
file containing:This results in the following on Windows (with Windows cmd, CygWin bash or VSCode bash):
where I would expect it to echo
my-nice-app
.Is it a known and accepted limitation or a bug?
The text was updated successfully, but these errors were encountered: