Fork me on GitHub
#shadow-cljs
<
2022-03-12
>
p-himik19:03:22

Just updated shadow-cljs to the latest and started getting this warning:

[To redirect Truffle log output to a file use one of the following options:
* '--log.file=<path>' if the option is passed using a guest language launcher.
* '-Dpolyglot.log.file=<path>' if the option is passed using the host Java launcher.
* Configure logging using the polyglot embedding API.]
[engine] WARNING: The polyglot context is using an implementation that does not support runtime compilation.
The guest application code will therefore be executed in interpreted mode only.
Execution only in interpreted mode will strongly impact the guest application performance.
For more information on using GraalVM see .
To disable this warning the '--engine.WarnInterpreterOnly=false' option or use the '-Dpolyglot.engine.WarnInterpreterOnly=false' system property.
Adding -Dpolyglot.engine.WarnInterpreterOnly=false does remove it in its entirety. But that's only possible because I use deps.edn. Would people using shadow-cljs.edn for their dependencies also be able to suppress the warning? Maybe this warning something that shadow-cljs can suppress itself?

thheller19:03:23

no clue what that warning is. never seen it before.

thheller19:03:50

maybe you have a different graaljs engine in deps.edn? do you actually use graalvm maybe?

p-himik20:03:42

Nope. This is enough to trigger the warning:

{:paths ["src"]
 :deps  {thheller/shadow-cljs       {:mvn/version "2.17.8"}
         superstructor/re-highlight {:mvn/version "1.1.0"}}}
Removing superstructor/re-highlight removes the warning. That dependency doesn't have any CLJS dependencies - only deps.cljs that has this:
{:npm-deps     {"highlight.js"          "11.1.0"}
 :npm-dev-deps {"react"                 "17.0.2"
                "react-dom"             "17.0.2"
                "shadow-cljs"           "2.15.1"
                "karma"                 "6.3.4"
                "karma-chrome-launcher" "3.1.0"
                "karma-cljs-test"       "0.1.0"
                "karma-junit-reporter"  "2.0.1"}}

thheller20:03:47

the warning is triggered by the graaljs engine. which I guess works differently when used on actual graalvm

thheller20:03:07

the graaljs engine is used when comparing :npm-deps versions in deps.cljs files

thheller20:03:37

so I guess the re-highlight has one and removing that will not cause that part of the code to run

thheller20:03:54

I may just remove that check altogether or make a manual command to run

thheller20:03:05

its usefulness is rather limited anyways

p-himik20:03:21

Is the version comparison there only to warn people about inconsistent versions?

thheller20:03:54

its used to warn about conflicts in deps.cljs files

thheller20:03:08

ie. reagent wanting react v16 but something else wanting react v17 or whatever

thheller20:03:53

setting :npm-deps {:install false} in shadow-cljs.edn should also get rid of the warnings

p-himik20:03:22

But why is evaluating JS needed for that? Because of JS-based SemVer functionality?

thheller20:03:23

also won't isntall any js deps though ;)

p-himik20:03:26

A-ha, found semver.js. If I, say, rewrote that in CLJ, would you use that instead of evaluating semver.js on GraalJS?

thheller20:03:35

would be best to just implement that in clojure but npm versions are a complete mess and I really can't be bothered to figure out how it works šŸ˜›

thheller20:03:43

yes, absolutely. please do šŸ™‚

p-himik20:03:57

Heh, will try.

p-himik20:03:17

One caveat - it will be harder to update once SemVer inevitably becomes 3.0.0 or even 2.0.1.

thheller20:03:40

I could just remove it entirely or just bail on conflict

thheller20:03:22

pretty sure everyone just ignores the warning anyway šŸ˜‰

p-himik20:03:30

But you can't figure out whether there truly is a conflict without doing all that crap, can you? You can only check for not=.

p-himik20:03:37

Naah, I check those warnings. :)

thheller20:03:28

don't know. there are definitely deps.cljs with version ranges out there

p-himik22:03:06

God damn. That code is a complete mess, its semantics notwithstanding.

p-himik22:03:25

Actually, this raises a question - why didn't you use https://github.com/vdurmont/semver4j or https://github.com/zafarkhaja/jsemver?

thheller05:03:22

hmm guess I didn't know those exist?

thheller05:03:09

changing to those immediately šŸ˜›

thheller06:03:01

hmm neither has the necessary function for checking two ranges against each other

p-himik08:03:26

Yeah, but that function is just every? within some within every? within some. :)

p-himik08:03:12

On the other hand, neither of those two libraries seem to be well maintained, and there are some bad looking outstanding issues. But then again, node-semver seems to be in exactly the same boat... As an alternative, seems that you can use npm exec to run packages without having to install them - they will be fetched to an internal npm cache that won't interfere with anything, or at least it seems that way. The current CLI of node-semver doesn't allow for range intersection, but maybe it can be extended. At least this works:

npm exec --package=semver -- semver -r '1.0.0 - 2.1.0' 1.5.0

thheller08:03:30

this really doesn't seem worth the trouble overall

thheller08:03:38

I'll just remove it and pick the first one that is found

thheller08:03:47

and just warn about others in case of duplicates

thheller08:03:22

its not like thats any worse than what you currently get

p-himik08:03:07

One final alternative - you can use node to evaluate semver.js. If npm is installed, then node is installed, and I think it's possible to get its path reliably.

thheller08:03:31

forking a node process has other issues though. thats why I didn't do that to begin with

thheller08:03:17

again .. this is all not worth it. especially since it cannot be reconciled anyways in case of a real conflict

p-himik08:03:20

Ah, huh, alright. Is there a write-up of the issues somewhere, or maybe you could describe them in a sentence?

thheller08:03:41

can't do anything anyways if something wants react v16 when something else wants react v17 but react v18 is already installed

p-himik08:03:25

Yeah, but if there is no conflict but multiple ranges and you get rid of semver.js, you can't do anything but keep on warning, right? Even if the ranges are perfectly fine.

thheller08:03:36

there might be multiple :npm-deps declared dependencies that overlap or conflict. can only install one. which one do you pick?

thheller08:03:51

thats the problem

thheller09:03:08

could just trim down the version to something sane and compare that

p-himik09:03:16

What I mean is that one might require 1.0.0 - 2.0.0 and the other 1.1.0 - 1.9.0 while the installed version is 1.5.0. There's nothing to warn about, it's all consistent. But how can you check for it without semver.js?

thheller09:03:17

ignore versions altogether and just pick the higher one

thheller09:03:38

1.0.0 - 2.0.0 is not version range ever used in npm

thheller09:03:58

so ^17.0.0 or whatever

thheller09:03:07

just take out the ^ and compare against the other

thheller09:03:16

if higher install that however it was originally

thheller09:03:39

it never checks against an installed version

thheller09:03:58

it really is just about the automated npm install for :npm-deps found in deps.cljs files

thheller09:03:13

if its already installed nothing happens, regardless of version. that check was removed ages ago since it was never correct (ie. broken builds didn't warn, completely fine builds did)

thheller09:03:06

the problem wouldn't exist if nobody used version ranges in :npm-deps but some do

thheller09:03:27

actually some "did". haven't looked in many years what it looks like nowadays

p-himik09:03:43

How come npm does not use ranges, but :npm-deps allows it?

thheller09:03:07

what do you mean? npm uses ranges by default all over the place?

p-himik09:03:28

> 1.0.0 - 2.0.0 is not version range ever used in npm

thheller09:03:41

expressed in that format I mean

thheller09:03:30

its always some shit like ^2.0.1 or ~2.0.1. I don't even know what these all mean and really don't care enough to figure that out

thheller09:03:14

but there are more complex ones out there and those are the real problem

thheller09:03:47

best option is probably to make this an interactive command

thheller09:03:55

ie. shadow-cljs install-npm-deps or something

p-himik09:03:09

Ah, well - then my point still stands. :) Surely some JS packages use explicit ranges with both bounds. So if I understand you correctly, there are 3 approaches: ā€¢ Keep parsing and comparing ranges, via semver.js or whatever else ā€¢ Just install the highest version and hope for the best (but how will you know which one is the highest without parsing the ranges?) ā€¢ Install something according to one found version range and warn if there are other version ranges that aren't byte-for-byte the same

thheller09:03:16

that on conflicts presents all options and asks for answer

thheller09:03:50

yes, plus what I just said

p-himik09:03:54

In the case of an interactive command, it would put an extra onus on the user - and probably 90% of all users don't even need to run it, and another 90% won't even know about the command.

thheller09:03:15

well it could just warn on startup if things are missing

thheller09:03:50

"react was found as a declared dependency but is not install. please run npx shadow-cljs install-npm-deps." seems fine to me

p-himik09:03:09

Alright. Suppose I run that command and now nothing is missing. But then I add a new package that requires a completely different versions of already installed packages. You won't be able to warn about it, and I won't be able to realize that everything is broken.

thheller09:03:40

that is fine. there is no check for this now. installed packages always win.

thheller09:03:47

can't change this behavior either because thats the way you resolve conflicts

thheller09:03:01

you install one that should win

thheller09:03:20

can't just keep installing a newer version that the user may not want

thheller09:03:22

there could also be a npx shadow-cljs audit-npm-deps

thheller09:03:33

the problem really is just in trying to automate all this stuff too much

thheller09:03:53

conflict resolution just needs input from the user to be resolved

thheller09:03:22

the only feedback I ever got on this feature is "how do I turn off this warning"

p-himik09:03:24

> can't just keep installing a newer version that the user may not want Right, but could keep at least warn the user. It's an interesting difference in approaches, unless my knowledge is incorrect. npm just won't let you install something that doesn't satisfy the ranges unless you --force it. Maven/`deps.edn` will just resolve everything and use whatever's available, without complaining. The user will have to explicitly run a command to check if there are any conflicts.

thheller09:03:53

npm install will install anything without any checks

thheller09:03:12

if it runs into a conflict on transitive deps it will install them in nested folders

thheller09:03:16

not resolving them at all

p-himik09:03:03

Eh, feedback is lopsided. Of course nobody will tell you "this warning is helpful, thanks for adding it". > npm install will install anything without any checks Uhm... Just a couple of weeks ago I couldn't install MUIv5 because of a conflict. I think it was requiring React 17 but some other lib really needed 16.

thheller09:03:15

well maybe nowadays there are warnings. can't say. I don't use many npm dependencies anymore (really just one)

thheller09:03:52

hmm yeah dunno. never saw that myself

thheller09:03:48

gotta go. feel free to open an issue about this if you have suggestions or want to implement semver.js šŸ˜‰

p-himik09:03:07

> I don't use many npm dependencies anymore Heh, kinda ironic. But also enviable in a way. Yeah, will think a bit more about it first.

p-himik09:03:10

Oh, there might be a very simple and easy solution. You can concatenate versions with spaces, it won't break anything since there's only one binary operation, ||, and it has the highest precedence - the spaces are implied as &&. So something like >1 <8 >2 >3 || <1 is a valid version string. So: 1. Gather all :npm-deps 2. merge-with them with concatenating the version strings with spaces (replace empty versions with "*") 3. Add to those the contents of package.json, but only for dependencies that were in :npm-deps 4. Run npm install for every accumulated dependency with an explicit version If there are conflicts, npm will loudly fail. If there are no conflicts, the required dependencies will be installed, if they haven't been installed before.

mkvlr19:03:57

though one issue is that older graal versions don't support the flag and will crash even if you bring in the script engine as a dependency and that would support this flag, been meaning to file an issue about this

agold19:03:07

Why am I not able to load source map?

DevTools failed to load source map: Could not load content for : HTTP error: status code 404, net::ERR_HTTP_RESPONSE_CODE_FAILURE

thheller19:03:30

status code 404 suggests that your :asset-path is likely wrong

thheller19:03:50

oh wait. that is not a file from a shadow-cljs build?

thheller19:03:09

so I guess the file is just missing?

agold21:03:52

The :asset-path is correct. The file is clearly missing, but I have no idea what is attempting to load it. What is supposed to generate that .map file?

thheller21:03:36

I don't know. might be your html? might be something else you are including? this is not a file from a shadow-cljs build and as such shadow-cljs also wouldn't generate a .map file for it

agold22:03:44

OK, thanks.

ribelo22:03:23

has anyone encountered a similar warning?

Receiver reference in function ribelo.fatum.Fail changes meaning when namespace is collapsed.
Consider annotating @nocollapse; however, other properties on the receiver may still be collapsed.