Fork me on GitHub
#shadow-cljs
<
2022-02-20
>
mhuebert02:02:03

I’ve run into a bug (quite some months back) with macros in self-host where syntax-quoted symbols aren’t always expanded. Found some time today… I came across this bit here where we build up an alias-map and bind that to reader/**alias-map*,* but then override `reader/resolve-symbol` to ana/resolve-symbol which doesn’t appear to have access to those aliases directly (though the analyzer is building up aliases in its own way, i presume) https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/build/compiler.clj#L312

mhuebert02:02:44

the symbol that doesn’t expand is sub/wrap

mhuebert02:02:52

now, weirdly, this only happens if there is both a macros.cljs and macros.cljc file. if I delete the .cljs file and use only .cljc with the self-require there, the symbol expands as expected.

mhuebert02:02:29

maybe it’s because with the cljs file present, in the first “pass” there are no :require’s found by the analyzer

mhuebert02:02:23

whereas with a self-requiring cljc file, it finds the sub-macros namespace on the first (non-macros) pass

mhuebert02:02:35

so I guess that is my theory, that the read-loop in shadow’s compiler builds up an alias map which is not visible to the analyzer namespace, but is required for syntax-quote expansion. however sometimes it “works anyway” if the analyzer has already “seen” the same namespace in a prior pass, and the namespace-alias was also visible to that pass.

thheller05:02:01

isn't this a known problem? macro namespaces can't have dependencies?

thheller05:02:25

because it is undefined which namespace it should include?

thheller05:02:03

does this include the .cljs file or the .cljc file? it is technically speaking in a :cljs branch of the .cljc file but loading in CLJ mode since it is a macro

mhuebert08:02:18

If it's not behind a reader conditional then I'd expect it to load / be a available in all passes

mhuebert08:02:24

This is a macro using another macro

mhuebert08:02:59

Something related to this behaviour changed several months ago (or more) and I haven't been able to update Maria.cloud’s dependencies since then because it breaks a lot of macros

mhuebert08:02:06

Eg Reagent.core/with-let used to work but doesn't anymore because it uses an ns alias in a macro to delegate to a macro in another namespace

mhuebert08:02:16

In this case I know that shadows compile loop is aware of the alias from logging at that point

mhuebert08:02:03

The code is compiled and available, it's just the syntax quote expansion that doesn't work

mhuebert08:02:33

If I replace the use of the alias with the fully qualified symbol it works

thheller09:02:19

as far as I can tell you are in undefined territory and macros cannot be included this way

thheller09:02:33

this might be fine in .clj files and just .cljc being the problem?

thheller10:02:37

so if I diagnose this correctly the problem is that requires in CLJ files are not followed

thheller10:02:52

(that includes requires in the CLJ path of the CLJC files)

thheller10:02:10

so as far as the compiler is concerned nothing ever requires the sub-macros

thheller10:02:20

if you instead however properly expose that require it all works out

thheller10:02:37

ie. add

(ns bootstrap-test.macros
  (:require-macros bootstrap-test.macros)
  (:require [bootstrap-test.sub-macros]))

thheller10:02:00

self-host is mind bending enough already, if you add CLJC to the mix it all becomes even worse

thheller10:02:51

dunno if that actually fixes anything though. just a theoretical observation

mhuebert10:02:23

But isn't it the case that these requires are being followed? Because it works if I change the symbol to be fully qualified. It is only the read phase where the syntax quote doesn't work

mhuebert10:02:17

And it seems weird to be building up a map of ns-aliases but then overriding resolve-sym so that it can't see them

thheller10:02:32

what makes you say that it can't see them?

thheller10:02:47

ana/resolve-symbol is the only thing that can see them?

mhuebert10:02:57

They are bound in the reader ns, not ana?Ana is only seeing stuff from previous passes? If I log the ns-aliases I see one for sub but that's not being seen or used by ana/resolve-symbol

mhuebert10:02:47

When I'm back at my computer I can point to the line

thheller10:02:45

ana/resolve-symbol sees things in the analyzer data. any ns data is added to that

mhuebert10:02:55

Line 311 of shadow.build.compiler

mhuebert10:02:47

I can log there that we encounter a syntax quote sub/wrap symbol, sub is found in ns-aliases but resolve-symbol doesn't find it

mhuebert10:02:19

bootstrap-test.macros does a self :require-macros but in a cljs file separate from the cljc file

thheller10:02:56

but isn't this because it is self-host compiling with this reference context.

(ns bootstrap-test.macros
  (:require-macros bootstrap-test.macros))
so there is no sub alias?

thheller10:02:03

shouldn't it work if you add it here?

thheller10:02:11

I'm kinda lost with self-hosted and cljc to be honest

thheller10:02:15

never makes sense to me

mhuebert10:02:07

But shouldn't aliases work when read in the same pass?

mhuebert10:02:48

This is also broken (ra alias not resolving)

mhuebert10:02:54

Though I haven’t had time to verify it's exactly the same cause

mhuebert10:02:19

IIRC I thought these issues were shadow specific, couldn't repro in the cljs test suite

mhuebert10:02:31

But I checked that a few months ago

mhuebert10:02:28

But that's what also makes me think it could be a quirk in shadow.build.compiler

mhuebert11:02:27

so it’s the same reason why i can’t compile reagent’s macros

mhuebert11:02:14

the macro pass’s syntax-quote can’t read its ns-aliases

thheller11:02:31

I mean it is absolutely possible this is a problem in shadow-cljs code somewhere

thheller11:02:16

but to me this all looks like it is working as intended? this issue with using fully qualified names for macros has been there since forever (and even exists in non-self-hosted code)

mhuebert11:02:33

something definitely changed with this several months ago though because this broke a bunch of things with Maria

thheller11:02:57

that is possible since I added support for :as-alias a couple months ago. maybe that messed something up?

mhuebert11:02:02

it was earlier than that

mhuebert11:02:44

it was already broken in november 2020

mhuebert11:02:21

maria is still on shadow-cljs 2.8.36

mhuebert11:02:31

i git bisected it a long time ago but can’t find the result

mhuebert11:02:51

i’m just trying to “unfreeze” maria now so i can work on it again

thheller11:02:58

hmm yeah would help to know the version number it breaks

mhuebert11:02:07

there were 2 bugs in ClojureScript that had to be fixed first

mhuebert11:02:11

but those are all taken care of now

mhuebert11:02:46

i know there are many cases where one needs to use fully qualified symbols in macros but this I think is different - these are symbols for macros, from macro namespaces that are required by a macro namespace. eg https://github.com/reagent-project/reagent/blob/master/src/reagent/core.clj#L10 this works as expected

mhuebert11:02:33

(but is broken for me)

mhuebert11:02:58

of course one has always had to use fully qualified namespaces for stuff that is calling at runtime

mhuebert11:02:48

but this makes me think, maybe im wrong about the main issue being syntax quote expansion

mhuebert11:02:07

(i wouldn’t think so)

thheller11:02:09

as far as I can tell the same problem exists in 2.8.36?

thheller11:02:32

what do you mean supposed to extend macros?

thheller11:02:30

sorry, gotta go. bbl

mhuebert11:02:46

ach now i remember how hard it was to bisect this, i can’t even run the old version anymore because of java incompatibilities

mhuebert11:02:04

[:failed-to-compare "^1.2.0" "1.2.2" #error {
 :cause "Cannot invoke \"Object.getClass()\" because \"target\" is null"
 :via
 [{:type java.lang.NullPointerException
   :message "Cannot invoke \"Object.getClass()\" because \"target\" is null"
   :at [clojure.lang.Reflector invokeInstanceMethod "Reflector.java" 97]}]

mhuebert13:02:26

well, looks like reagent has other reasons why its not selfhost-compatible so that’s not a good goal. I will see again how many macros broke that we were actually using in maria

thheller14:02:49

those errors you can ignore. they don't affect compilation

thheller14:02:09

so as far as I can tell your repro also breaks in 2.8.36, so this issue isn't the one affecting maria I guess?

mhuebert23:02:36

well I apologize for this rabbit trail. When I made those original repro’s I think I assumed that since shadow and cljs appeared to diverge here it was also a bug that was probably contributing. However, I’ve finally fixed all the other things that had broken in the meantime and all our curriculum runs fine without any changes needed to this macro-syntax-quote stuff.

mhuebert23:02:35

I think I have still run into this issue elsewhere when trying to write selfhost-compatible macros. But I’ve mostly given up on that because it’s so hard.

mhuebert23:02:10

I did verify that a :refer in a .cljs file makes an alias available during the macro pass of a .cljc file - as you described above

Jacob Emcken06:02:44

I found requiring Hero Icons like this also includes the Hero icons not referred:

(:require ["@heroicons/react/solid" :refer [CheckIcon ChatIcon]])
While the following does not:
(:require ["@heroicons/react/solid/CheckIcon" :as CheckIcon]
          ["@heroicons/react/solid/ChatIcon" :as ChatIcon])
Is there any way to require without repeating "@heroicons/react/solid" over and over? I tried with:
(:require ["@heroicons/react/solid" ["CheckIcon" :as CheckIcon]
                                    ["ChatIcon" :as ChatIcon]])
Which causes: Invalid namespace declaration

thheller06:02:41

no, that is not supported

👍 1
Jacob Emcken06:02:45

Is it something that is impossible to do with how such things work or is it just something not important enough to get any attention?

thheller07:02:15

changing :require to allow the nested stuff is not an option since that would break compatibility with standard CLJS

thheller07:02:55

changing that :refer only actually imports what was referred is a substantial amount of work, which I simply don't have time for currently. it is also non-standard behavior and basically something webpack invented.

Jacob Emcken07:02:25

Thanks for the insight. I'm just curious 😄

Jacob Emcken07:02:28

JS tooling is something I've successfully managed to dodge for a very long time... exactly because examples like "non-standard behavior and basically something webpack invented".

thheller07:02:05

yeah its annoying

barrell09:02:21

Hey thheller - general question for you. Is it possible to read the metadata for clojurescript functions from build hooks (which are in clojure)? Or is it generally possible to parse the namespace and get publics ~ trying to figure out the best way to inspect a clojurescript file from the build and determine some general information about it

thheller09:02:05

sure, all analyzer data is available

thheller09:02:33

[:compiler-env :cljs.analyzer/namespaces 'your.ns :defs 'foo :meta] in the build state

1
barrell09:02:33

this is amazing. you’re the best.

Quentin Le Guennec10:02:21

Hello, I'm getting an error in a release build (works in dev mode): $.clearTimeout is not a function the corresponding call is here:

goog.functions.debounce = function(f, interval, opt_scope) {
  'use strict';
  let timeout = 0;
  return /** @type {function(...?)} */ (function(var_args) {
    'use strict';
    goog.global.clearTimeout(timeout);
    const args = arguments;
    timeout = goog.global.setTimeout(function() {
      'use strict';
      f.apply(opt_scope, args);
    }, interval);
  });
}; 

thheller11:02:14

which :target is this and what is it actually running in?

Quentin Le Guennec11:02:57

I'm using npx shadow-cljs release main --debug for :target = :browser

thheller11:02:25

and the output is running in an actual brower?

Quentin Le Guennec11:02:49

my deps.edn is:

{:paths ["src/main" "resources"]
 :deps  {org.clojure/clojure    {:mvn/version "1.10.3"}
         com.fulcrologic/fulcro {:mvn/version "3.5.9"}}

 :aliases {:dev {:extra-paths ["src/dev"]
                 :extra-deps  {org.clojure/clojurescript {:mvn/version "1.10.914"}
                               thheller/shadow-cljs      {:mvn/version "2.16.9"}
                               binaryage/devtools        {:mvn/version "1.0.4"}
                               cider/cider-nrepl         {:mvn/version "0.27.4"}}}}}

Quentin Le Guennec11:02:58

I pulled it from the fulcro guide

thheller11:02:59

with that being the only JS on the page?

Quentin Le Guennec11:02:10

it's webpack-ed though

thheller11:02:23

well ... that is the problem then

Quentin Le Guennec11:02:39

I'll try to release a version without webpack

thheller11:02:45

the output of :browser is not supposed to be bundled by webpack again?

thheller11:02:59

and why would you do that?

Quentin Le Guennec11:02:53

isn't it a good practive?

thheller11:02:23

I don't know what you are trying to accomplish by doing so?

thheller11:02:36

I mean does it have a reason you are doing it? is there something missing otherwise?

Quentin Le Guennec11:02:41

I thought it would reduce the loading times

thheller11:02:00

no, it will in fact make it worse

Quentin Le Guennec11:02:22

I need the bundle at least for css files though, is that fine?

thheller11:02:58

I mean you can run webpack separately. that is fine as long as it doesn't try to process the shadow-cljs output

Quentin Le Guennec11:02:21

yeah, it should be fine just for css then

Quentin Le Guennec11:02:54

I'd like to avoid having to manually add copy css files from node_modules/ to my public directory]

mauricio.szabo15:02:42

Hi there! I'm trying to import a npm dependency, and I'm getting the error IllegalArgumentException No matching field found: getAbsolutePath for class clojure.lang.PersistentHashMap. I know it's probably an error with the way I'm importing the lib, but is there something else I can do to debug this issue?

thheller17:02:55

@mauricio.szabo which npm dependency would this be? you can also provide the full stacktrace and shadow-cljs version please. that would already telling me about 95% of what I need to know

mauricio.szabo17:02:40

It's the monaco-editor, the full require is ["monaco-editor/esm/vs/editor/editor.api" :as monaco]. This is the stacktrace on the shadow-cljs console:

IllegalArgumentException No matching field found: getAbsolutePath for class clojure.lang.PersistentHashMap
        clojure.lang.Reflector.getInstanceField (Reflector.java:397)
        clojure.lang.Reflector.invokeNoArgInstanceMember (Reflector.java:440)
        shadow.cljs.devtools.errors/fn--13467 (errors.clj:228)
        shadow.cljs.devtools.errors/fn--13467 (errors.clj:222)
        clojure.lang.MultiFn.invoke (MultiFn.java:239)
        shadow.cljs.devtools.errors/error-format (errors.clj:449)
        shadow.cljs.devtools.errors/error-format (errors.clj:440)
        shadow.cljs.devtools.errors/error-format (errors.clj:443)
        shadow.cljs.devtools.errors/error-format (errors.clj:440)
        shadow.cljs.devtools.server.worker.impl/build-failure/fn--14276 (impl.clj:133)
        shadow.cljs.devtools.server.worker.impl/build-failure (impl.clj:132)
        shadow.cljs.devtools.server.worker.impl/build-failure (impl.clj:128)

thheller17:02:26

thanks. yeah thats a bug, fix incoming

thheller17:02:35

my guess its trying to include .css files

thheller17:02:58

fixed in 2.17.4

mauricio.szabo18:02:57

Is an import from a css file even valid? Why does Monaco is trying to import it? facepalm

mauricio.szabo18:02:57

@U05224H0W is there a way to ignore these CSS files when I compile my code with Shadow-CLJS?

thheller18:02:46

:js-options {:ignore-asset-requires true}

👍 1
thheller18:02:58

but usually packages that have these expect to be bundled by webpack

thheller18:02:04

so other stuff might break too

Quentin Le Guennec21:02:30

what's the best way to import css files from node_modules into the shadow-cljs build in the browser? I tried webpack but it's not working

ag00:02:06

have you looked into postcss? I'm currently using tailwind with postcss, and importing tailiwind CSS modules directly from node_modules.

Quentin Le Guennec09:02:04

Thanks for your answer. Does it also import other assets (like fonts) like react does natively?

ag10:02:10

It's been a while since I touched anything front-end related, and I've never before worked with Tailwind. Also, it's been a long time since I used postcss. I'm sorry I can't give you a detailed answer. afaik tailwind has its own way of dealing with fonts. https://tailwindcss.com/docs/font-family and postcss has plugins https://www.postcss.parts/tag/fonts?searchTerm=font