subpath exports are supported, but were a pretty recent addition. maybe you are on an older shadow-cljs version?

Sorry for the delay — taking a look now. I have updated to the most recent version 2.25.10. I’m stopped with a new error, so I do not know if the version update fixed it. The new error occurs after running watch:

[build-cljs:dev] Failed to inspect file
[build-cljs:dev]   /Users/kenny/work/kwllc/deeporigin/data-mgmt/projects/ui/node_modules/@tanstack/query-core/build/modern/focusManager.cjs
[build-cljs:dev]
[build-cljs:dev] Errors encountered while trying to parse file
[build-cljs:dev]   /Users/kenny/work/kwllc/deeporigin/data-mgmt/projects/ui/node_modules/@tanstack/query-core/build/modern/focusManager.cjs
[build-cljs:dev]   {:line 30, :column 2, :message "'}' expected"}

#focused;

var FocusManager = class extends import_subscribable.Subscribable {
  #focused;
  #cleanup;
  #setup;
  constructor() {
    super();
...

those are unfortunately still not supported by the closure compiler https://github.com/google/closure-compiler/issues/2731

unfortunately nothing I can do about that from the shadow-cljs side. you can use webpack (or any other JS bundler) via https://shadow-cljs.github.io/docs/UsersGuide.html#js-provider-external

Ah got it, ok. Given the upcoming maintenance mode of the closure compiler, do you think it’s pretty unlikely this will get in?

the closure library is going into maintenance mode

I don't think the closure compiler is. that would surprise me.

Ah, phew - ok. Thanks

Hmm curious - this library provides a “modern” and “legacy” build. Would I be able to use the “legacy” build for just this library? It seems to not use the class field syntax.

The focusManager code snippet looks like this in the legacy build.

var import_utils = require("./utils.cjs");
var _focused, _cleanup, _setup;
var FocusManager = class extends import_subscribable.Subscribable {
  constructor() {
    super();
    __privateAdd(this, _focused, void 0);
    __privateAdd(this, _cleanup, void 0);
    __privateAdd(this, _setup, void 0);
    __privateSet(this, _setup, (onFocus) => {

well ... the package appears to be using "exports" for the modern version

https://unpkg.com/browse/@tanstack/[email protected]/package.json

so you could turn of exports, which in turn breaks the other package again 😛

so no, not really. apart from manually changing that package.json of course

change line 29 to build/legacy/index.cjs and it might work

It feels naughty to modify something in node_modules 😅

hehe

It seems like the webpack workaround is fairly straightforward. Thank you for the clear writings on it 🙂

I’m somewhat surprised I’ve never run into this in the many years of cljs work. I suppose it’s somewhat new. Are folks tending to transition to it now? Or did I just get “lucky” with this tanstack library?

its come up a couple of times recently. for whatever reason the JS world jumps on any new syntax feature as soon as webpack supports it

even it the standards aren't even out or finished yet

the more trendy the library the more cutting edge the code 😛

Circling back - I’m writing an internal note about this issue since I’ll likely tackle in in the next couple weeks. Any guess on why upgrading shadow from 2.23.3 to 2.25.10 causes the class field error? i.e., why wasn’t that error thrown before?

the older shadow-cljs didn't support "exports", thus it took the legacy version of the tanstack thing

I mean I guess I could add an option to :ignore-exports #{"tanstack"} or something to ignore it selectively, but not too sure thats a good idea

"exports" is another thing that is getting quite more common and relied upon

This is also a relatively new feature, right?

hmm not that new anymore no, been a couple years now

not many packages used it though, so didn't appear often until recently

Did webpack recently add support for it? 😄

I think it was like 2020 or so

whenever webpack5 was released

Hmm. I was wondering if the two were linked — increased use of subpath exports means more likely more code to use the class field syntax, haha

exports kinda make sense, although its a total mess

class fields, or especially the "private" syntax it total nonsense

looks stupid and breaks every parser. gotta love the JS world for breaking changes

convention for private used to be _foo, now its #foo. totally worth breaking the syntax for that

Do they write justification on why a change like that is worth making that tradeoff?

reasons are not part of the standard process I think 😛

well some blame is also on the closure compiler folks. I don't get why just parsing the # and discarding it is not a valid path forward 😛

don't know why this is taking so long to implement

For now I can just rename it to something else

but then I have to touch lots of source code

probably related to this? https://github.com/thheller/shadow-cljs/blob/45587a80bf55bb971ed009168a80af58a0fd02ac/src/main/shadow/build/compiler.clj#L411-L417

I guess I didn't account for anyone making their own defmacro? 😛

no worries, I can work around it ;)

happy to make that smarter and respect :refer-clojure :exclude

yeah I think logging an issue and making it more robust makes sense

shall I log one?

sure

https://github.com/thheller/shadow-cljs/issues/1156

out of curiosity though. what does this do that makes it relevant to be called in CLJS?

I mean since macros run in their own world, keeping defmacro in the regular CLJS world serves no purpose other than creating bugs

or is this not related to what actual defmacro is and something entirely different?

it defines a SCI macro

(defn add-macro-args [[args body]]
  (list (into '[&form &env] args) body))

(clojure.core/defmacro defmacro [name & body]
  (let [[?doc body] (if (and (string? (first body))
                            (> (count body) 2))
                      [(first body) (rest body)]
                      [nil body])
        bodies (if (vector? (first body))
                 (list body)
                 body)]
    `(defn ~(vary-meta name assoc :sci/macro true)
       ~@(when ?doc [?doc])
       ~@(map add-macro-args bodies))))

So it keeps the macro around as a normal function with two extra arguments and I can more or less just copy existing macros to a .cljs file which then work in SCI