Fork me on GitHub
#cljs-dev
<
2020-11-06
>
mhuebert12:11:37

after a discussion with @thheller (https://clojurians.slack.com/archives/C03S1L9DN/p1604658107070900) about some self-host macro issues I’ve run into, I found something that I think I can reproduce in cljs itself - https://github.com/clojure/clojurescript/compare/master...mhuebert:cljc-macros-require-test - does that look like it should work?

mhuebert12:11:34

I was running ./script/test-self-host on an older version of clojurescript, after rebasing master I’m seeing fixed via script/bootstrap

SEVERE: /Users/mattmini/Documents/projects/clojurescript/builds/out-self/cljs/core.js:27: ERROR - Closure dependency methods(goog.provide, goog.require, etc) must be called at file scope.
cljs.core._STAR_target_STAR_ = goog.define("cljs.core._STAR_target_STAR_","default");
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

thheller12:11:20

might need to run script/bootstrap again in case that is out of date

thheller12:11:28

not actually sure thats still relevant though

thheller12:11:09

but the error above comes from mixing incompatible closure-compiler/closure-library versions

mhuebert13:11:10

@thheller thanks, that was the reason why I couldn’t run the tests after updating

mhuebert13:11:16

weird thing about that failing test: it is in a group of 4 related tests. if I move it to the last position in the group (after (ns foo.bar)\n(first [1 2 3])), it passes

mhuebert13:11:46

what voodoo. if I just copy it so it’s in both places (with a latch of 5), then both tests pass

mhuebert13:11:12

But without the copy at the end, fails

thheller13:11:27

maybe some hidden race condition somewhere?

mhuebert11:11:24

so it looks like in selfhost cljs, calls to (ns …) blow away existing ns data, which is different from clojure, where you can use (ns foo.bar) to switch namespaces without removing existing mappings. this can explain the race conditions in tests that I was seeing. (not sure if it’s related to the original problem of unresolved aliases in macro namespaces.) https://github.com/mhuebert/clojurescript/pull/1/files#diff-7144cb9420ccfd2b33c6c96777c668bba2046694d2431c231d67357fc15008efR856

dnolen14:11:46

@braden.shepherdson that's why we don't update GCL or Closure Compiler very often

dnolen14:11:51

there's always breaking changes

dnolen14:11:08

and both compiler and library must be coordinated - it's really annoying

dnolen14:11:41

also generally there's often not super meaningful changes over the span of years

dnolen14:11:47

as far as ClojureScript is concerned

dnolen14:11:13

that said, it's on my plate to look at in the 3-4 weeks, and certainly before the end of the year

Braden Shepherdson14:11:22

hm. I'm likely to discover other breaking changes, then, if I'm using master GCL and jscompiler.

dnolen14:11:45

patches welcome of course, but auditing everything that could go wrong is actually way more trouble than you may understand

dnolen14:11:53

i.e. downstream tooling might break

dnolen14:11:11

so even if we fix some stuff it might all be delayed

dnolen14:11:32

until it's communicated and agreed what is required

Braden Shepherdson14:11:37

I'll fiddle with it locally, and at least file a bug with the details. this AnonymousFunctionNamingPolicy case specifically is an option being dropped, so it's easy enough to just stop setting it. I guess it's possible that there's downstream tooling that depends on setting it to something non-default, though I think this is a pretty obscure option.

dnolen15:11:27

yes, please capture that one and anything else you encounter

dnolen15:11:06

I will also give it a try - Figwheel will also need a spin, I need to test Krell, yadda yadda

dnolen15:11:13

so the urgency is quite low

thheller15:11:36

FWIW I took a look yesterday the offending code can just be removed. I doubt anyone ever used the related setting since source maps were introduced.

dnolen15:11:50

sure, I'm just pointing out that testing the new compiler/library takes some time now - it might be easy - might not - if things are working in shadow w/o other changes that's a good sign o' course

thheller15:11:59

can't do much in shadow since everything breaks when cljs.closure is loaded

thheller15:11:51

but shadow is on v20200830 without issues. I believe the september update just broke things because of the removal.

thheller15:11:05

otherwise will probably work just fine

mhuebert11:11:24

so it looks like in selfhost cljs, calls to (ns …) blow away existing ns data, which is different from clojure, where you can use (ns foo.bar) to switch namespaces without removing existing mappings. this can explain the race conditions in tests that I was seeing. (not sure if it’s related to the original problem of unresolved aliases in macro namespaces.) https://github.com/mhuebert/clojurescript/pull/1/files#diff-7144cb9420ccfd2b33c6c96777c668bba2046694d2431c231d67357fc15008efR856