babashka-sci-dev

2022-04-08T19:08:53.532349Z

Afternoon. Has anyone tried adding new dependencies to nbb and noticed different behavior between dev and release compiled versions? Adding datascript and seeing d/q fail for some unexpected cases

borkdude 2022-04-08T19:10:29.503339Z

What is the unexpected error? Maybe an advanced compilation error?

2022-04-08T19:12:17.534419Z

Nothing fails during compilation. When I run this test ns, https://github.com/tonsky/datascript/blob/master/test/datascript/test/query.cljc, half of the tests fail in advanced mode but they pass fine in dev mode

borkdude 2022-04-08T19:13:50.299719Z

That's not what I meant. I mean if you notice a difference between dev and prod and then it might be an error as the result of advanced compilation

borkdude 2022-04-08T19:14:53.338999Z

if you compile with --debug you might be able to see what goes wrong in the case (if it is that)

2022-04-08T19:18:50.313499Z

Ah gotcha. The buggy release build definitely seems caused by advanced compilation. Will try --debug

2022-04-08T19:20:52.996339Z

If you're curious, here's the branch - https://github.com/babashka/nbb/compare/main...logseq-cldwalker:add-datascript?expand=1 . Steps to repro test failure:

$ NBB_FEATURES=datascript,datascript-transit bb compile
# Tests pass
$ node lib/nbb_main.js test-scripts/datascript-test/query.cljc
$ NBB_FEATURES=datascript,datascript-transit bb release
# Tests fail
$ node lib/nbb_main.js test-scripts/datascript-test/query.cljc

2022-04-08T19:33:18.974139Z

With NBB_FEATURES=datascript,datascript-transit bb release --debug --verbose didn't see any obvious failures or culprits. My next troubleshooting steps are: • see if it's an externs issue. Dunno how good shadow's :auto inference is • Look into the ns behind d/q , https://github.com/tonsky/datascript/blob/master/src/datascript/query.cljc, and see if any of the underlying namespaces aren't behaving as expected by running tests for the underlying namespaces I'm open to troubleshooting other ways too

borkdude 2022-04-08T19:36:27.968839Z

Is it doing anything with other JS libs?

2022-04-08T19:43:18.690299Z

Don't see any other npm/js lib for datascript. It's only other external dep is https://github.com/tonsky/persistent-sorted-set/blob/master/src-clojure/me/tonsky/persistent_sorted_set.cljs

borkdude 2022-04-08T19:44:33.687509Z

Have you used datascript in other advanced compiled projects before?

2022-04-08T19:46:10.000249Z

It's been awhile but used to use it a fair amount on browser targets. Don't know if I've used it against nodejs

borkdude 2022-04-08T19:48:24.263759Z

then what's the actual error you're getting

2022-04-08T19:51:36.312819Z

Failed assertions when I run node lib/nbb_main.js test-scripts/datascript-test/query.cljc e .g.

FAIL in (test-q-in) (/Users/me/code/work/nbb/test-scripts/datascript-test/query.cljc:99)
expected: (= (d/q query db :name "Ivan") #{[1] [3]})
  actual: (not (= #{} #{[1] [3]}))
No explicit exceptions. Would be much more helpful if I had that

borkdude 2022-04-08T19:54:29.217929Z

And this does work in dev mode?

2022-04-08T19:58:19.779519Z

yep. works fine. Interestingly enough, this issue so far seems specific to d/q behavior. These two other test namespaces pass fine in dev and advanced mode - https://github.com/tonsky/datascript/blob/master/test/datascript/test/query_rules.cljc and https://github.com/tonsky/datascript/blob/master/test/datascript/test/pull_api.cljc

borkdude 2022-04-08T20:08:36.698029Z

interesting

borkdude 2022-04-08T20:08:52.756299Z

and also specific to usage within SCI or also happening outside?

2022-04-08T20:29:03.802049Z

Looks like this is a known issue - https://github.com/tonsky/datascript/wiki/Tips-&-tricks . I'm able to manually modify the advanced compile js per one of the linked issues. I should eventually be able to get this right with a shadow compiler option 🙏 Thanks for chatting with me. Sometimes a quick chat helps unblock us. This is all the more motivating to get this working with nbb. I don't want logseq users and most cljs users to have to mess with compiler options for esm to be productive with datascript and cljs. 😄 For integration tests, I see the ci:test task runs off of one release build. Should I add another release build with features enabled or just reuse the existing one to also test datascript?

borkdude 2022-04-08T20:30:40.342479Z

Maybe you can fork nbb and activate the tests in the CI of that fork?

borkdude 2022-04-08T20:33:42.998669Z

Similar to bb, it doesn't run tests for features in CI because it would take way too long

2022-04-08T20:33:51.084029Z

That works too. I didn't know if you wanted to test that features worked in some way but happy to run them in my fork

borkdude 2022-04-08T20:35:04.384879Z

Maybe this can be done through env vars as well or so

borkdude 2022-04-08T20:35:14.546119Z

Then you'd only have to change your CI env settings

👍 1
borkdude 2022-04-08T20:35:26.748029Z

and no code diffs, that would be optimal

2022-04-08T20:38:10.591949Z

Meaning no whitespace changes in PRs or something else? The recent clojure.test -> cljs.test was helpful. The only manual changes for datascript tests I have to do now are change ExceptionInfo to Error

borkdude 2022-04-08T20:38:37.229539Z

We could add ExceptionInfo

borkdude 2022-04-08T20:38:48.601799Z

I mean, that all of the feature code lives in the central repo

👍 1
borkdude 2022-04-08T20:39:02.909509Z

but to kick off the tests for your fork which you are going to distribute I assume, you run the tests

borkdude 2022-04-08T20:39:13.570449Z

without changing the code

2022-04-08T20:39:32.024549Z

Ah. That would be nice

2022-04-08T20:41:27.790539Z

> We could add ExceptionInfo Would be happy to do this. Would I add it under :classes in sci/init?

borkdude 2022-04-08T20:41:38.776419Z

I think so

borkdude 2022-04-08T20:42:15.716399Z

not sure, since it lives in the cljs.core namespace

borkdude 2022-04-08T20:42:38.900569Z

Maybe for now yes, just in classes

👍 1
borkdude 2022-04-08T20:42:44.174379Z

would probably make most of the code work

borkdude 2022-04-08T20:43:59.913059Z

is that class used like (instance? ExceptionInfo ...)?

2022-04-08T20:46:08.674219Z

Used with a try/catch by thrown-with-msg? testing expression

borkdude 2022-04-08T20:49:00.640259Z

right