Fork me on GitHub
#squint
<
2022-12-30
>
chaos18:12:50

Hi, how is the source-maps npm dev dependency used in squint? I'm getting a native compilation error when trying to install it with npm install on MS-Windows with VS-Code 2022/node.js 19.3.0. There isn't much information about it at https://www.npmjs.com/package/source-maps nor there's a public repository I can raise an issue for.

chaos18:12:41

(Nevertheless I reported the issue to the author by email)

borkdude18:12:05

@chaos not sure if it's used at all

borkdude18:12:37

we should add some windows tests...

chaos18:12:17

Shall I remove it? I'm about to submit a PR about a REPL issue I've just raised, and also thinking of adding win coverage to CI

chaos18:12:14

(The tests pass without the source-maps dev dep btw)

borkdude18:12:43

@chaos please make a PR to compiler-common - once it's committed there, it will be mirrored in the squint repo. I'm thinking of organizing things differently, but this is how it's currently set up

borkdude18:12:02

yes, please remove it - I'll add it back when it's necessary and deal with the Windows problem myself

chaos18:12:33

also, the squint/node.js file is picked up as the node executable when trying to invoke node (since js is a valid executable extension in win) 😅shall I rename it to nodex.js or suggest a better name? 🙂

borkdude18:12:37

this file is part of the distribution...

chaos18:12:56

ok not sure what this means; the issue manifest itself when I run bb build, which invokes (shell "npx shadow-cljs --config-merge .work/config-merge.edn release squint") which results in opening squint/node.js in visual studio code (my guess here is that npx or shadow-cljs invokes node and node.js is executed instead)

borkdude18:12:49

The meaning is that node.js is part of the squint-cljs package on npm and contains the Node.js API

borkdude18:12:01

What would be a better way to do this? node_api.js?

chaos18:12:22

as in rename it to node_api.js ? it looks sensible to me

borkdude18:12:49

let's do that...

borkdude18:12:21

I've looked into better ways of offering a Node.js vs browser API in index.js but it required transpilation / webpack, etc, stuff I didn't feel like using

chaos18:12:48

I suppose the only other place I need to reflect the change is in package.json, there is mention of it under "files"

chaos19:12:42

Do I need to change both compiler-common/.github/.../squint.yml and compile-common/squint/.github/.../ci.yml actions or just the first?

borkdude19:12:31

both, for now

👍 1
borkdude19:12:25

I think I'll just move compiler-common to squint and then depend on squint from cherry, so I'll cut out compiler-common as the middle man

chaos20:12:01

Why is again having compiler-common as a git submodule in both not a good option?

borkdude20:12:21

because I'd like to execute tests when changes are PR-ed against compiler-common

chaos20:12:42

is it too difficult to trigger an update from a compiler-common gh action to cherry and squint for it?

chaos20:12:18

I think clojure-lsp has a bot that issues a PR to clojure-lsp every time a new dependency version is created that seems akin to the above

chaos20:12:10

so I was thinking, perhaps, when a compiler-common commit is made a action is triggered to create a PR for squint and cherry to bump the submodule to that version. The PR will trigger the tests for both.

chaos20:12:12

Just a thought

borkdude20:12:25

also sounds complex. I've just spent literally two days doing the thing there is now and I'm still not satisfied

chaos20:12:11

ah I see though, this is about the compiler-common PRs not merges to master, it will create a lot of noise to cherry and squint while the PR is being worked on. Another idea perhaps is to treat squint and cherry as "integration" tests; the compiler-common PR action could clone cherry and squint and update their compiler common submodule to that of the PR, then run their tests

chaos20:12:06

From my perspective it's fine as it is now btw, once you realize that you have to work on the compiler-common repo and not in squint (it took me some time to figure this out, perhaps the squint's readme.md needs to be more specific if the current scheme remains in place). The only gray area is the CI that needs to be duplicated by hand and run twice on GH

borkdude20:12:19

that sounds like a good idea, but sometimes you will make changes both compiler-common and squint, and then that gets annoying too

borkdude20:12:44

(this was a reply to your first message before mine, not the second)