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

🐛 BUG: wrangler pages dev -- npm run serve gives an "Unknown argument" error #965

Closed
MAXOUXAX opened this issue May 11, 2022 · 29 comments · Fixed by #1057 or #1025
Closed

🐛 BUG: wrangler pages dev -- npm run serve gives an "Unknown argument" error #965

MAXOUXAX opened this issue May 11, 2022 · 29 comments · Fixed by #1057 or #1025
Labels
bug Something that isn't working pages Relating to Pages

Comments

@MAXOUXAX
Copy link

What version of Wrangler are you using?

2.0.2

What operating system are you using?

Windows

Describe the Bug

I am currently giving WebStorm a try and I am encountering a problem when running the command below:
wrangler pages dev --proxy 8080 -- npm run serve

Indeed, when this command is executed via a classic Command Prompt, everything works fine, but when it is executed from PowerShell, the following error is displayed:
error

The command npm run serve works correctly.

@MAXOUXAX MAXOUXAX added the bug label May 11, 2022
@petebacondarwin
Copy link
Contributor

petebacondarwin commented May 11, 2022

I just tried this and could not reproduce the error. Steps I took:

(Windows 10 - PowerShell %SystemRoot%\system32\WindowsPowerShell\v1.0\powershell.exe)

  • mkdir tmp
  • cd tmp
  • npm i -D wrangler
  • Update package.json with "scripts": { "serve": "echo FOO" }
  • npx wrangler pages dev --proxy 8080 -- npm run serve
🚧 'wrangler pages <command>' is a beta command. Please report any issues to https://github.com/cloudflare/wrangler2/issues/new/choose
Running npm run serve...
No functions. Shimming...
Serving at http://localhost:8788/
[proxy]:
> serve
> echo HERE


[proxy]: HERE

X [ERROR] Proxy exited with status 0.

@petebacondarwin
Copy link
Contributor

Can you provide a runnable example of the problem?

@petebacondarwin petebacondarwin added the pages Relating to Pages label May 11, 2022
@petebacondarwin petebacondarwin added the needs reproduction Needs reproduction from OP label May 12, 2022
@Flotss
Copy link

Flotss commented May 13, 2022

Can confirm this is a bug since I am also having the same issue.

Using [email protected] and:

  • getting the exact same error when running the same command on [email protected]
  • getting no error when using the Command Prompt or Git Bash

I have this problem when I try to run the command just after cloning this project (so you can try by yourself): https://github.com/Flotss/maxouxax.me

@Flotss
Copy link

Flotss commented May 13, 2022

  • npx wrangler pages dev --proxy 8080 -- npm run serve

I just tried starting wrangler with npx, and the command works just fine when doing so.
The command only fails when running wrangler using the wrangler command.

@MAXOUXAX
Copy link
Author

MAXOUXAX commented May 13, 2022

  • npx wrangler pages dev --proxy 8080 -- npm run serve

I just tried starting wrangler with npx, and the command works just fine when doing so. The command only fails when running wrangler using the wrangler command.

The behavior is very strange since I just tried to run the command with and without npx, and both commands failed.

@petebacondarwin petebacondarwin modified the milestone: Pages May 15, 2022
@petebacondarwin petebacondarwin moved this to Untriaged in workers-sdk May 15, 2022
@petebacondarwin petebacondarwin added bug Something that isn't working and removed type: bug labels May 16, 2022
@MAXOUXAX
Copy link
Author

Can you provide a runnable example of the problem?

👉 Here is a complete reproduction video from scratch, including PowerShell, npm and NodeJS versions

wrangler_issue.mp4

@petebacondarwin
Copy link
Contributor

Hi @MAXOUXAX - thanks for these reproduction steps. I am finally able to get the same problem and I think I have a workaround. The problem is that yargs is only looking for a single positional argument after the --. But it sees npm run serve as three separate positional arguments; and so it is complaining that serve is not a valid positional argument.
You can get this to work by putting the command in double quotes: npx wrangler pages dev -- "npm run serve".

@petebacondarwin
Copy link
Contributor

I think I also have a fix which is very simple:

https://github.com/cloudflare/wrangler2/blob/7a191a2fd0cb08f2a80c29703a307286264ef74f/packages/wrangler/src/pages.tsx#L1247

needs to change to:

"dev [directory] [-- command..]",

telling Yargs that command is variadic.

@petebacondarwin petebacondarwin removed the needs reproduction Needs reproduction from OP label May 18, 2022
petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue May 18, 2022
On Windows, the following command `wrangler pages dev -- foo bar` would error
saying that `bar` was not a known argument. This is because `foo` and `bar` are
passed to Yargs as separate arguments.

A workaround is to put the command in quotes: `wrangler pages dev -- "foo bar"`.
But this fix makes the `command` argument variadic, which also solves the problem.

Fixes [cloudflare#965](cloudflare#965)
@petebacondarwin petebacondarwin self-assigned this May 18, 2022
@MAXOUXAX
Copy link
Author

Glad you successfully reproduced the bug!

Although I never built a CLI and therefore never used yargs, the bug only happens in PowerShell, the command works just fine in other prompts, which would mean that yargs behaves differently between prompts, which is pretty odd, and looks like a bug to me.

Just to be sure I got that right, it's not an issue coming from wrangler, but an non-intended behavior coming from yargs, which needs to be fixed, right?

Thanks for the workaround though, will use this for now!

@petebacondarwin
Copy link
Contributor

I am not sure why the behaviour is different between command prompts. I guess different prompts pass the args in in different ways at the OS level?? Anyway, my PR (#1057) should be a reliable fix across all, and is arguably the right way to do this anyway.

petebacondarwin added a commit to petebacondarwin/wrangler2 that referenced this issue May 18, 2022
On Windows, the following command `wrangler pages dev -- foo bar` would error
saying that `bar` was not a known argument. This is because `foo` and `bar` are
passed to Yargs as separate arguments.

A workaround is to put the command in quotes: `wrangler pages dev -- "foo bar"`.
But this fix makes the `command` argument variadic, which also solves the problem.

Fixes [cloudflare#965](cloudflare#965)
threepointone pushed a commit that referenced this issue May 19, 2022
On Windows, the following command `wrangler pages dev -- foo bar` would error
saying that `bar` was not a known argument. This is because `foo` and `bar` are
passed to Yargs as separate arguments.

A workaround is to put the command in quotes: `wrangler pages dev -- "foo bar"`.
But this fix makes the `command` argument variadic, which also solves the problem.

Fixes [#965](#965)
@MAXOUXAX
Copy link
Author

MAXOUXAX commented May 19, 2022

I think the issue needs to be reopened, because the command wrangler pages dev -- npm run serve is completely broken on PowerShell.

What happens is that it doesn't execute the command passed as a parameter, so it opens a web server that tries to proxy requests on port 8080, but fails completely because there's no web server on that port, since the given command hasn't been executed.

image

EDIT: I thought the command was broken for all prompts, but it actually works in all prompts except PowerShell. So the problem is simply not solved.

@threepointone
Copy link
Contributor

Can you try with wrangler 2.0.6, does the problem persist?

@MAXOUXAX
Copy link
Author

Can you try with wrangler 2.0.6, does the problem persist?

Oops, didn't mention I, of course, upgraded to [email protected]!
As you can see in the screenshot, the command works fine, it's just not throwing an error anymore, but rather starting Wrangler and nothing else.

@petebacondarwin
Copy link
Contributor

Let me try this again...

@petebacondarwin
Copy link
Contributor

OK so the problem is that the yargs definition is dev [directory] [-- command..], where both directory and command are optional. In PowerShell it appears that yargs will place the first positional string (even after the --) into the directory option. So if you do: dev -- npm run serve you end up with directory === "npm" and command === "run serve" 😢
And even if you do dev -- "npm run serve" then you get directory === "npm run command" and command === undefined 😢

@petebacondarwin
Copy link
Contributor

So here is the problem...

In Windows Powershell when I use npx I am seeing:

process.argv = [ "node.exe", "path/to/wrangler/cli.js", "pages", "dev", "--proxy", "8080", "npm run serve"]

In Windows Command Prompt when I use npx I am seeing:

process.argv = [ "node.exe", "path/to/wrangler/cli.js", "pages", "dev", "--proxy", "8080", "--", "npm run serve"]

Spot the difference?

This seems to be something related to npx since if I run a simple node.js script:

node index.js foo -- bar

where index.js looks like:

console.log(process.argv);

Then I do see the -- in the process.argv...

@petebacondarwin
Copy link
Contributor

I think we might need to record this as a known issue in Pages + Powershell?

@MAXOUXAX
Copy link
Author

You can get this to work by putting the command in double quotes: npx wrangler pages dev -- "npm run serve".

I just want to mention that even this solution doesn't work anymore since [email protected], so my current solution is to run the npm run serve command separately after running the wrangler command.

@petebacondarwin
Copy link
Contributor

I think this is the best workaround for now. @MAXOUXAX - did the original workaround (to use quotes) actually work with 2.0.5? I believe it would not have, since even with out the fix in 2.0.6 it would still have the same problem.

@MAXOUXAX
Copy link
Author

MAXOUXAX commented May 20, 2022

Haven't tested the workaround in previous versions so cannot really tell

@petebacondarwin petebacondarwin removed their assignment May 23, 2022
@petebacondarwin
Copy link
Contributor

Unassigning me for the time being so the Pages team can make a call on what to do next.
If Powershell support for this is mandatory, then it might be that we move to a named option (e.g. --command="...") rather than relying upon the post -- positional argument support.

@Skye-31
Copy link
Contributor

Skye-31 commented Aug 5, 2022

Just tried this behaviour on wrangler 2.0.24, and I am unable to reproduce. Are you still experiencing it? This may also be due to my more up to date powershell version.

Get-Host | Select-Object Version

Version
-------
7.1.5

@MAXOUXAX
Copy link
Author

Just tried this behaviour on wrangler 2.0.24, and I am unable to reproduce. Are you still experiencing it? This may also be due to my more up to date powershell version.

Get-Host | Select-Object Version

Version
-------
7.1.5

Can confirm this is still not working, wrangler spins up the web server, but the npm command never gets executed.
Please see the screenshot below which includes the PowerShell version, the wrangler version, and the NodeJS version
image

@jrf0110
Copy link
Member

jrf0110 commented Oct 12, 2022

@MAXOUXAX The Pages team does not plan to support this sort of command-chaining in Powershell in the near future. As such, we are closing this issue.

@jrf0110 jrf0110 closed this as completed Oct 12, 2022
@MAXOUXAX
Copy link
Author

@MAXOUXAX The Pages team does not plan to support this sort of command-chaining in Powershell in the near future. As such, we are closing this issue.

Alright, just a few things then:

  • Tried again with the newest version of wrangler (2.1.11) AND a new computer and the problem persists
  • Running the exact same command but prefixing 'npx' fixes it, the command works just fine that way
  • Maybe add a warning somewhere that running wrangler in PowerShell may not work

@nneil
Copy link

nneil commented Apr 1, 2024

This doesn't seem to be too much to ask for. One command below works and the other doesn't. It seems like a fix would be possible. Otherwise this need to be clearly spelt out in the documentation.

Works

npx wrangler pages dev --proxy 8080 -- npm run dev

Fails

wrangler pages dev --proxy 8080 -- npm run dev

@petebacondarwin
Copy link
Contributor

@nneil - what version of wrangler do you have installed globally?

Try running:

npx wrangler --version
wrangler --version

The difference between the two commands you used is that the first will use the locally installed version and the second will use the global one. We do not recommend using a globally installed one; instead you should install wrangler local to your project and then use npx (or equivalent command for other package managers) to run it.

@nneil
Copy link

nneil commented Apr 2, 2024

@petebacondarwin That's a good point, but I should have been clearer. I install nothing as a global: I would much rather waste some disk space and have precise control over my versions. I just put ./node_modules/.bin in my path so I don't need npx

PS> Get-Command wrangler

CommandType     Name                                               Version    Source
-----------     ----                                               -------    ------
ExternalScript  wrangler.ps1                                                  C:\Users\nneil\Projects\cloudflare\node_modules\.bin\wrangler.ps1

@petebacondarwin
Copy link
Contributor

Ah, right. This is an issue with how Powershell works, not the version of Wrangler that you are running.
Sorry I should have actually checked the thread of the issue before responding.

This is a known issue on a Pages command that we have deprecated. I think it would be nice to try to tell if you are trying to do this in PowerShell and add a warning. I'll open an issue to add that.

Otherwise, there is no plan to make this work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that isn't working pages Relating to Pages
Projects
None yet
7 participants