Not the first time something "unexpected" happens inside the Vega builds. I'll file an issue there about this too, but I don't have high hopes of getting it fixed unless someone understands their build system well enough and digs into it

https://github.com/vega/vega-embed/issues/780 for those who are affected

yeah, their entire build system seems rather messed up. no clue why this is in the released builds, it can't serve any purpose

var scripts$1 = {
    prebuild: "yarn clean && yarn build:style",
    build: "rollup -c",
    "build:style": "./build-style.sh",
    clean: "del-cli build build-es5 src/style.ts",
    prepublishOnly: "yarn clean && yarn build",
    preversion: "yarn lint && yarn test",
    serve: "browser-sync start --directory -s -f build *.html",
    start: "yarn build && concurrently --kill-others -n Server,Rollup 'yarn serve' 'rollup -c -w'",
    pretest: "yarn build:style",
    test: "beemo jest --stdio stream",
    "test:inspect": "node --inspect-brk ./node_modules/.bin/jest --runInBand",
    prepare: "beemo create-config",
    prettierbase: "beemo prettier '*.{css,scss,html}'",
    eslintbase: "beemo eslint .",
    format: "yarn eslintbase --fix && yarn prettierbase --write",
    lint: "yarn eslintbase && yarn prettierbase --check",
    release: "auto shipit"
  };

seems to import the entire package.json just to get the version out

That indeed seemed to be the gist of their latest release

I've gotten the feeling that perhaps build systems aren't their forte, but I don't know the background on how they then chose the tools they are using right now

A shame, really. The library has/had great potential, but I guess is severly lacking just contributors/maintainers in general, or at least those well versed in JS tooling

Where do I have to crete this module.export ?

@hoertlehner you could just use "vega-embed": "6.19.1", in your package.json if you don't really need anything in the newest release. @U05224H0W and @U8ZQ1J1RR thanks for looking into this. Not the greatest start with vega, but it was recommended as a good starting point for charting, so I'm going to stick with it a while and see if issues like this are frequent.

I rather see it as the end point, not the starting point. It's pretty heavy duty. But that's a discussion for another thread (and channel).

I fixed it by including this version. But I would like to know how I could fix it via the module.export.

I am using vega-embbed not directly, but I use react-vega

So I now have one additional dependency moure in my project.

So I would rather fix it with some kind of hack.

And who knows, it might be, that a new error appears in the future.

vega breaks OFTEN

@hoertlehner I think you can just include an iterator.js file with the contents module.exports = function() {};. https://shadow-cljs.github.io/docs/UsersGuide.html#_patching_libraries might be helpful.

thanks a lot @alex.sheluchin

yeah you just put a iterator.js in the directory of the vega build file

./iterator.js from /home/florian/repo/gorilla/goldly/profiles/bundelci/node_modules/vega-embed/build/vega-embed.js

means it was trying to find /home/florian/repo/gorilla/goldly/profiles/bundelci/node_modules/vega-embed/build/iterator.js but couldn't since it does not exist

but how can I add a directory to a npm temporary directory? I would need to first know when cljs watch has finihed getting the npm dependencies, so I could then manually add this file.

I think if you just put the file in a mirrored location somewhere on your classpath, it should get picked up correctly when building.

shadow-cljs watch doesn't get any npm dependencies ever

at most it is installed when you start shadow-cljs

but if you run npm install just before everything should already be ready and shadow-cljs won't install anything

so you have the opportunity to modify whatever before shadow-cljs even runs

but ... best option to me seems to not use a broken release?

It's not an extra dependency anyway if it's already a subdependency of something you use.

it would pick up a js file on the classpath?

Ah, no, that's my mistake. You would need to put it next to the vega-embed.js file. But yeah, best option is to just roll back to a working release.

Coming back to this, would the module version of the vega-embed work better? That is, if you have :js-options {:entry-keys ["module" "browser" "main"]} in the build target.

https://unpkg.com/browse/[email protected]/build/vega-embed.module.js seems to be packaged completely differently and doesn't seem to try to import the missing file

I’m guessing it’s not possible or simple to assume any node module is JS and automatically infer that anything imported from it is ^js?

I cannot see your code so I don't know what you are talking about specifically

for npm imports this is already done yes

I’ll try to come up with a minimal reproduction case and post here when I do. In any case from what I can tell it seems that the same code, compiled using these different targets, leads to different inferences and at least in this case code that should be inferred at JS isn’t in the :npm-module target. I assume it’s because, as you mention, the :browser target processes the JS code while the :npm-module target doesn’t

just an example of the warning you get helps to infer what you are talking about

I mean just adding the hints should solve it?

Type hints would solve it but we don’t need them for our current app code (we don’t get warnings for the :browser compilation target) since the automatic externs inferring works in that case so it’s not a solution we would prefer to use if possible

why not? they won't hurt the browser build and will in fact make it more reliable given that the compiler doesn't have to guess anymore.

Well, primarily because we don’t need them for the browser build. In any case I will make a minimal repro repo and share with you here later

@U05224H0W So we talked about adding type hints internally and are okay with it. I went ahead and tried to do it based on my understanding of how it should work and I think I uncovered a bug, but need more input - long story short in :target :npm-module target, if you use a JS function in a binding, and then later use that result in an anonymous function, it doesn’t seem to keep track of it even with a type hint and you’re forced to add a type hint in the anon function. https://github.com/jo-sm/shadow-externs-repro if you want to take a look, I tried to describe it as clearly as I can and am open to any ideas you have

this is somewhat expected. you are welcome to make a enhancement patch for CLJS but as of now externs inference is not that smart

but also kind of ambiguous. objects is a JS value as hinted (so probably an array) but you are map'ing over it and it does not automatically imply that each item in that array is also a JS value

could be an array of CLJS maps, there is no way for the compiler to know this

Sure, but the issue doesn’t appear with the :target :browser mode (even with externs inferring fully disabled), and it doesn’t get fixed with type hints as I understand them from the official reference. So I feel like something isn’t working as expected

An alternative would be to just silence the warnings using :compiler-options {:warnings {:infer-warning false}} but I am also struggling to get that to work for :npm-module

Anyway, if you have any ideas how to get around this let me know. The simplest approach would be to just shut the warnings up but it doesn’t seem to work (if I’m misplacing or misspelling the option, let me know), but adding type hints is also viable, but it also seems to work in an unexpected way (if my repro and understanding are correct)

everything is working as intended. you found the correct place to add the typehint. just do exactly that.

externs inference is never fully disabled

you are only disabling the warnings

sure, but disabling the warnings seems to do nothing

and disabling the warnings will most likely lead to broken builds so just don't do it

the warnings still appear in the terminal

in this specific case disabling the warnings is acceptable as this is for tests, not for the main build of our application

imho you should still run your tests using :advanced release builds. still might catch issues in your main build

I’m a bit confused still - externs and :advanced aside, is it possible to disable warnings for :npm-module?

you can try setting :js-options {:generate-externs false} in your :browser build config

maybe that gets you do the same place as :npm-module

I’m still missing the answer to my question 😅

:compiler-options {:infer-externs false} should turn off warnings

ok

let me try it

you can also set (set! *warn-on-infer* false) in a file after the ns or so to turn off all warnings in that file only

but I repeat that the warnings are there for a reason and you probably spent more time talking to me about that problem than adding a few ^js would take

ah ok so :infer-externs false does indeed fix it. Does it work differently than just disabling the warning (or does disabling the warning not work as I expect, which is that it won’t output it if it encounters it)?

yes it'll still do the exact same inference. just not warn about it.

disabling the warning likely just conflicts with the other setting defaulting to be enabled and overriding that setting

Yeah it’s not that we are definitely going to disable the extern warnings - as you said they’re there for a reason - it’s more that we want to have the option to. In this specific case they’re being compiled for tests where we would expect the tests to fail for other reasons and this warning isn’t as relevant (unlike for the main build where it is more relevant). I was more trying to figure out why exactly the way that it seems like it should work according to the docs (adding :warnings {:infer-warning false} ) wasn’t

Fixed by first deleting the .shadow-cljs directory that must have become corrupted. Doubt it had anything to do with the OS upgrade.

shadow-cljs - connected to server is the hint. the server was already running

by deleting .shadow-cljs you now likely have a zombie process thats no longer accessible. make sure to kill it manually.

ps -aux | grep shadow-cljs | less . Thanks.