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

jack-in generates invalid shadow-cljs call #2239

Closed
zilti opened this issue Jul 3, 2023 · 15 comments
Closed

jack-in generates invalid shadow-cljs call #2239

zilti opened this issue Jul 3, 2023 · 15 comments
Labels

Comments

@zilti
Copy link

zilti commented Jul 3, 2023

Calva generates an unsupported shadow-cljs call for project type shadow-cljs, see this issue report on the shadow-cljs repository. For some reason, the generated command line still works on macOS and Linux, but it does not work on Windows.

@PEZ PEZ added the jack-in label Jul 3, 2023
@PEZ
Copy link
Collaborator

PEZ commented Jul 3, 2023

Hi! Can you share the steps you take to get that command line generated? I don't think know how to make it add clj aliases...

@zilti
Copy link
Author

zilti commented Jul 3, 2023

I can, yes, here it is:

"calva.replConnectSequences": [
    {
      "name": "redsky Server",
      "projectType": "deps.edn",
      "cljsType": "none",
      "menuSelections": {
        "cljAliases": [
          "dev/deps",
          "dev/env"
        ]
      }
    },
    {
      "name": "redsky Server + Client",
      "projectType": "shadow-cljs",
      "cljsType": "shadow-cljs",
      "menuSelections": {
        "cljAliases": [
          "dev/deps",
          "dev/env"
        ],
        "cljsLaunchBuilds": [
          "app",
          "portfolio"
        ],
        "cljsDefaultBuild": "app"
      }
    }
  ]

This works on Linux and MacOS, but not on Windows, for some reason.

Alternatively, I also tried to use "projectType": "deps.edn" instead, but for some reason, that does not work - even though, when choosing "deps.edn + shadow-cljs" in the jack-in menu and selecting the exact same aliases and builds, it works fine. So something fails there when "scripting" it. Or I'd have to do something different to achieve the same.

@PEZ
Copy link
Collaborator

PEZ commented Jul 3, 2023

Thanks! Which version of Calva are you using?

@zilti
Copy link
Author

zilti commented Jul 3, 2023

According to the output when running it, it's v2.0.374.

@PEZ
Copy link
Collaborator

PEZ commented Jul 5, 2023

Hi again, sorry for the radio silence. I've hadn't had the time yet to try reproduce this. I'm pretty sure I can and that we will be able to fix the issue pretty easily.

Meanwhile: @thheller mentioned, in that other issue, that you can provide the clj aliases in the shadow-cljs file. You would change from :deps true in shadow-cljs.edn to :deps ["dev/deps", "dev/env"], and remove the "cljAlisases" configuration from that connect sequence. This should work around the issue.

@zilti
Copy link
Author

zilti commented Jul 5, 2023

Thank you a lot! Unfortunately, that workaround does not work for us, since we already have an alias in there, and adding dev/env would break our build due to it adding JVM startup options for dev only.

Anyway, we did find a different workaround for now, also mentioned by @thheller which is setting the customJackInCommandLine to

clojure -Sdeps '{:deps {cider/cider-nrepl {:mvn/version \"0.28.5\"}}}' -A:dev/deps:dev/env -M -m shadow.cljs.devtools.cli watch app portfolio.

@PEZ
Copy link
Collaborator

PEZ commented Jul 5, 2023

Thanks for updating about the workaround not working. Helps me prioritize the issue.

Great that you found a workaround. It should work with a custom command line like so too:

npx.cmd shadow-cljs -d cider/cider-nrepl:0.28.5 -A :dev/deps:dev/env watch app portfolio

It's a matter of preference where you like the shadow-cljs output to go.

In that other issue you mentioned that deps.edn + shadow-cljs didn't work for you. Can you file an issue about it? I'd like to try reproduce it and fix that too.

@PEZ PEZ mentioned this issue Jul 14, 2023
13 tasks
@PEZ
Copy link
Collaborator

PEZ commented Jul 14, 2023

@thheller
Copy link
Contributor

thheller commented Jul 15, 2023

I don't think this is going to work, and likely broke all other hosts too?

args seems to be supposed to be an array and each element corresponding to one argument. But now the "-A :foo" result is one string. If the node execution works like any other (e.g. JVM) that will end up getting escaped and arrive as one argument on the shadow-cljs end. I didn't actually test, but based on the fact that there is as args array getting constructed I presume the desired correct result should be ["-A", ":foo"] in the array, not ["-A :foo"].

@PEZ
Copy link
Collaborator

PEZ commented Jul 15, 2023

Thanks for having a look, @thheller! 🙏 It generates this command line on Mac:

pushd /Users/pez/Projects/calva/test-data/projects/shadow-w-backend ; npx shadow-cljs -d cider/cider-nrepl:0.28.5 -A :dev:dev-too watch app app-too ; popd

Which works. But you could very well be right that it doesn't help with the issue on Windows.

@thheller
Copy link
Contributor

The problem is that "printed" this command line appears totally normal, but at least on the JVM an arglist of ["-A" ":dev:dev-too"] will have a different meaning than ["-A :dev:dev-too"]. That might very well only be on issue on Windows, I'm not sure but I'd recommend following the API properly and properly diving arguments in the array.

@PEZ
Copy link
Collaborator

PEZ commented Jul 15, 2023

Indeed. I now fired up my Windows test machine, and it was still failing there. Adding the flag and the aliases to the args array separately works. Thanks again for the holler!

@PEZ
Copy link
Collaborator

PEZ commented Jul 15, 2023

I updated the docs around this a bit, as it was a bit unclear. They now look like so:

https://deploy-preview-2254--calva-docs.netlify.app/connect/#shadow-cljs-in-full-stack-projects

I hope they make more sense now...

@PEZ
Copy link
Collaborator

PEZ commented Jul 16, 2023

Fixed in 2.0.239.

@PEZ PEZ closed this as completed Jul 16, 2023
@zilti
Copy link
Author

zilti commented Jul 18, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants