Fork me on GitHub
#cljs-dev
<
2020-04-24
>
frozar09:04:07

@dnolen Hi, I would like to discuss (again I know) about the :default require keyword. I followed the issue at: https://clojure.atlassian.net/browse/CLJS-2376 And I understand why you are not inclined to support it in cljs compiler. In my situation, I'm working around the cljdoc project, and some of the analysed package contains the :default keyword in their requirement. In this case, the cljdoc-analyzer, which relies on the cljs parser facilities, just throws an exception during its execution. My question is: Is the support of :default keyword in the cljs parser definitely rejected? Or is it still in the balance? A strategy should be adopted in cljdoc from there to deal with :default keyword. (ping @martinklepsch)

thheller09:04:05

I typically advise people not to use :default in libraries so maybe the lib authors should just use the alternative :rename {default foo} instead of :default foo?

frozar09:04:22

Why not, it's possible. It's a way to deal with it. So the :default keyword from shadow-cljs is deprecated?

thheller09:04:38

no, it isn't. just makes libs shadow-cljs only which I lib authors should avoid (IMHO)

frozar09:04:09

Ok, if you advice to avoid it, so if cljdoc doesn't support this keyword, it's pretty reasonable, no?

martinklepsch09:04:00

Yes, I think supporting :default would be outside the scope for cljdoc since the library would be only usable from shadow anyways

frozar09:04:29

Ok, I have to fix my lib πŸ˜›

dnolen10:04:00

one alternative for the :default thing just always do the rename bit for Node.js libs?

dnolen10:04:32

Could be done for ES6 libs too of course if we detect them.

dnolen10:04:16

The thing I wanted to avoid with CLJS-2379 is more new stuff in the ns form - but if we just automatically do this would avoid the need for that

dnolen10:04:21

similar to what we do for invokeable nses

thheller11:04:52

what do you mean by automatic? I mean how would you know whether a react-native package has a default export or not?

dnolen12:04:18

right we would need to add that metadata, I thought our node_modules processing looked at exports

dnolen12:04:26

or could be easily modified to do so

dnolen12:04:57

this doesn't seem very hard to do - i.e. Rollup

dnolen12:04:18

in anycase this is the only kind of solution I would find acceptable - nothing manual

dnolen12:04:51

happy to see someone take this one - I would say medium difficulty

dnolen12:04:11

not too many changes to ClojureScript once the parsing problem is solved

dnolen12:04:15

https://clojure.atlassian.net/browse/CLJS-2376 ticket updated, @juhoteperi I'm assuming you don't still want to be assigned to this one quite old

dnolen12:04:47

I'm planning on cutting a release today - haven't head any bad news and all the latest changes are just to support downstream tooling

dnolen13:04:15

1.10.738 being built now

Roman Liutikov13:04:45

hmmm, I'm seeing cljs.core/+, all arguments must be numbers, got [clj-nil number] instead for (s/coll-of ::menu-item :kind sequential? :min-count 1)

dnolen13:04:50

@roman01la make something minimal and file a ticket, would be nice to know if that's a regression of some kind since we messed w/ this stuff recently

dnolen13:04:45

@roman01la yeah that was definitely one of the patches clj-nil not considered a number type anymore

Roman Liutikov13:04:10

Am I doing something wrong here?

clj -Sdeps '{org.clojure/clojurescript {:git/url "" :sha "b79007367818f0d1567646f28f09e2de3450a99e"}}' -m cljs.main -v -re node

Execution error (FileNotFoundException) at clojure.main/main (main.java:40).
Could not locate cljs/main__init.class, cljs/main.clj or cljs/main.cljc on classpath.

Roman Liutikov13:04:26

I think that was my patch

Alex Miller (Clojure team)13:04:58

you're missing the :deps level

Alex Miller (Clojure team)13:04:11

clj -Sdeps '{:deps {org.clojure/clojurescript ....

mkvlr13:04:23

right clj -Sdeps '{:deps {org.clojure/clojurescript {:git/url "" :sha "b79007367818f0d1567646f28f09e2de3450a99e"}}}' -m cljs.main -v -re node

Roman Liutikov13:04:23

ah I see, thanks

dnolen13:04:53

@roman01la I'm going to revert your patch

dnolen13:04:08

if you could reopen the ticket and add that would be helpful

dnolen13:04:10

building 1.10.739

dnolen13:04:21

website updated with release post

dnolen13:04:53

@alexmiller I want to do a post after that is there an easy way to have a post come afterwards with AsciiDoc even though the date is the same?

Alex Miller (Clojure team)13:04:13

not to my immediate knowledge

Alex Miller (Clojure team)13:04:37

there's a date sort somewhere

Alex Miller (Clojure team)13:04:57

2009/05/17 17:58:44
a format like that might work to set an explicit time for the post

dnolen14:04:47

cool thanks

Alex Miller (Clojure team)14:04:56

that seems to sort and appear correctly for me if I use 2020-04-24 23:59:59

Alex Miller (Clojure team)14:04:11

but may just be coincidence, not totally sure

Alex Miller (Clojure team)14:04:27

do you want me to push current site or wait?

dnolen14:04:51

wrapping up the others posts

dnolen14:04:54

one moment

dnolen14:04:06

@alexmiller just added some missing updates - let me know when it's ready and I will post updates to various channels

dnolen16:04:35

bunch of issues around the js-iterable? predicate, took a few tries to sort that out

dnolen16:04:51

1.10.741 should be appearing relatively soon

dnolen16:04:11

I updated the website to reflect the new version

dnolen16:04:34

stepping away for a little bit I believe everything should be ready to go - please try 1.10.741 when it appears would love to address any straggling issues if they come up today

Alex Miller (Clojure team)16:04:27

@dnolen sorry, was heads down on other stuff, will push site

dnolen17:04:15

@alexmiller minor typo, another push whenever you can

dnolen19:04:29

@thheller I don't really understand your concerns about handling exports when stuff like this exists https://github.com/rollup/plugins?

dnolen19:04:12

given the years of prior art - this doesn't seem problematic to me, your comments so far seem to be ignoring this?

dnolen19:04:00

@alexmiller another bump on the site, typo in the webpack guide

dnolen19:04:07

seems like you could even just do this with Rollup.js directly

dnolen20:04:45

another possibility

dnolen20:04:32

but as I expected JS already had to work around the chaos

thheller20:04:14

@dnolen the point I have about default is that I want something that DIRECTLY corresponds to a JS feature that is here now and used in libraries and probably more to come in the future as more people move to ESM

thheller20:04:59

it isn't even about how we process that, it is about being able to access it in a user friendly manner which IMHO :default foo does better than :rename {default foo}

thheller20:04:21

I understand you don't want to add stuff to the ns form and I'm fine with that ... don't do it then

thheller20:04:38

but adding compiler magic or adding third party tools does not address this problem in any way whatsoever

dnolen20:04:05

the :rename thing is not so import

thheller20:04:11

it is just about " how do I refer to this thing that library exports" nothing more

dnolen20:04:13

how it is accomplished isn't meaningful

dnolen20:04:17

"3rd party" is irrelevant

dnolen20:04:22

Closure is 3rd party thing

dnolen20:04:38

how it is detected is an implementation detail

dnolen20:04:58

so when we discuss it that's what I'm interested in

dnolen20:04:09

not talking about irrelevant implementation details - but the end result

dnolen20:04:29

if you require a library that exports default it can be used as a namespace

dnolen20:04:38

w/o accessing a property

thheller20:04:55

what? no it cannot?

thheller20:04:13

how do you mirror import * as thing from "foo" then if :as is already "used for default"?

dnolen20:04:09

let's rewind a bit

dnolen20:04:14

before talking past each other

thheller20:04:25

import Thing, * as everything from "foo";

dnolen20:04:41

I think you actual problem w/ the proposal is that in JS you have a choice

dnolen20:04:59

and choosing anything flies in the face of being able to choose in JS

thheller20:04:12

I don't follow

dnolen20:04:29

let's stop talking about import

dnolen20:04:34

just about how we want to use X from CLJS

dnolen20:04:42

some thing exports default

dnolen20:04:51

in JS I don't have to talk about default when I import

dnolen20:04:04

I just get a ns-y thing

dnolen20:04:15

that would be preferred in CLJS

thheller20:04:21

I don't follow sorry. you mean you don't have to use the word default?

dnolen20:04:39

you don't have to use default when you import in JS

dnolen20:04:45

so why should you have to use it in CLJS?

thheller20:04:53

yes, because it has dedicated syntax for it?

thheller20:04:10

like literally import <defaultExport> from "foo"?

thheller20:04:21

import Thing, * as everything from "foo"; this is actual valid JS?

dnolen20:04:35

and does that fail if you don't export default?

thheller20:04:36

and would be (:require ["foo" :as everything :default Thing])

thheller20:04:29

I'm lost again? that import will fail if the library has no default export yes?

dnolen20:04:45

just trying to parse that expression

dnolen20:04:06

it's imports the default under one name, then all other exported symbols into a different bag called everything

thheller20:04:08

all I'm looking for is being able to express all the variants of import that the ESM spec has

dnolen20:04:34

yeah not necessarily interested in that

thheller20:04:37

import Thing, {foo, bar} from "foo" is also valid

dnolen20:04:44

because of impedance mismatch

thheller20:04:51

what does that mean?

dnolen20:04:04

adding more stuff to the ns form

dnolen20:04:09

that isn't Clojure stuff

dnolen20:04:18

that's the hard design constraint

thheller20:04:42

so host-interop doesn't matter? I mean we have :refer-macros and other stuff clojure doesn't have?

dnolen20:04:28

sure but old days, we reconciled that with sugar long ago for .cljc

dnolen20:04:32

nobody wants to go backwards

dnolen20:04:04

Clojure has some prior art here w/ harder things to import, inner classes in packages

dnolen20:04:23

what about foo$default and "foo$default" ?

thheller20:04:58

give me a full example please. not really sure what you mean? as a separate require?

thheller20:04:34

I don't like it. Just makes things more complicated to explain IMHO

dnolen20:04:58

without more elaboration that's not much of an argument

thheller20:04:29

see ES6 Import to CLJS Require so that people can look at a JS example of how to use a certain library

thheller20:04:37

and have a direct translation of how you do that in CLJS

dnolen20:04:55

sure but that's not compelling argument

dnolen20:04:10

anyways the end result is something that would get the same result w/o futzing w/ the ns form

thheller20:04:18

I cannot make a compelling argument if you have already decided that you are not going to add :default

dnolen20:04:32

I've consistently rejected adding another thing for a couple years πŸ™‚

dnolen20:04:39

not gonna change my mind now

thheller20:04:49

then I don't know why we are even talking

dnolen20:04:58

but open to feedback about stuff that doesn't mess w/ the ns form

dnolen20:04:40

the other thing about foo$default is that it's trivial to implement

dnolen20:04:44

also trivial to support downstream

dnolen20:04:35

so there's a compatible way that libs/apps can use w/o caring

thheller20:04:27

I don't like it. I think it is needlessly confusing when we can express this nicely with actual syntax data and not magic strings

dnolen20:04:52

your feedback has been heard - now many times πŸ™‚

dnolen20:04:57

anyways let other people chime in

dnolen20:04:50

foo$default and "foo$default" semantically would let us get at the default export as a namespace-y thing

dnolen20:04:14

there's prior art in Clojure inner class import as well as CLS bootstrap foo$macros

dominicm20:04:54

I don't think I'd have too much problem adding the $default syntax to that table. I find it weird to use a namespace directly as a variable, but I know that's already there.

slipset20:04:42

Being more of a clj than a cljs dev, I find the finer details of jvm interop something I encounter very seldom. Most things I need are wrapped in a (thin) clj wrapper. Reason I’m mentioning this is that maybe this is the case with cljs as well, and if so, maybe it doesn’t matter too much if the syntax is a bit idiosyncratic?

dominicm20:04:39

@slipset I wouldn't say that's too true. There's a large number of npm packages, and they cover a broad scope. We use a large number of npm packages currently.

lilactown20:04:49

my personal feelings right now is that it would behoove CLJS to pave the path to using standard JS

lilactown20:04:54

ESM is not a moving target anymore, I think that laying some concrete on how syntactically we can translate between the JS and CLJS ecosystem helps a lot with onboarding and helping people feel productive

lilactown20:04:32

I think some of this discussion is brought on by the fact that tools like webpack have been very liberal in parsing ESM imports in the past

lilactown20:04:16

import React from "react" does work sometimes but it’s actually not valid, it should be import * as React from "react" but tools like webpack just let you get away with the former

thheller20:04:56

I'd be careful with that statement. It is still not "final" on how CommonJS <> ESM is ultimately going to work

thheller20:04:29

so import React from "react" is actually correct as long as react only ships CJS code

thheller20:04:06

but since no browser even supports that option and likely never will its always going to stay a bundler thing

lilactown20:04:38

there was a twitter thread where they were really encouraging people to use the actual ESM ns form instead of the default form (that relies on CJS interop)

lilactown20:04:55

and that they would be moving the docs / create-react-app to reflect this

lilactown20:04:24

probably because they really want to ship ESM but can’t because the ecosystem is still so choatic

thheller20:04:40

again .. thats a bundler thing and webpack keeps changing their mind on this. they want to release it with strict-mode and have been talking about breaking changes for a while now ... yet still no webpack v5

thheller20:04:02

react doesn't ship ESM because they want to do stuff you can't do with ESM

thheller20:04:24

like this conditional require of the minified bundle https://unpkg.com/[email protected]/index.js

thheller20:04:41

you cannot do this with ESM .. you need import maps for that ... but that is yet another unfinished spec

lilactown20:04:36

I see. I misspoke

thheller20:04:08

really I have no hope for this ever to work out

thheller20:04:39

the only clear way out of this is to have everything as ESM ... but that is going to take ages ... so we live in chaos πŸ˜‰

lilactown20:04:39

my point was that JS devs do have to make a choice, based on the standards, and that bundlers have been inconsistent in their approach to all of this.

lilactown20:04:00

but we should pave the path to using standard JS

lilactown20:04:16

my personal dream would be that I could copy + paste an ESM into the source path of my CLJS project and it would Just Work:tm:, and things that require bundling (commonjs, other garbage from npm that requires transpiling) I can use shadow-cljs/rollup/whatever

❀️ 4
thheller20:04:50

you can do that today. just can't also mix in some random npm commonjs code (eg. react)

lilactown20:04:05

right, but the importing thing we’re talking about above comes into play right?

thheller20:04:16

if you have all ESM it does not no. the interop/mix is the issue. all ESM or all CJS is totally fine.

lilactown20:04:59

I thought y’all were talking about how to refer to default imports in an ESM

lilactown20:04:07

the actual point I’m circling around is that I agree with adding a :default import πŸ˜„

thheller20:04:10

you can still use it regardless of :default. It really is just syntax sugar for :refer (default) :rename {default foo}. can't remember if the :refer was actually necessary so may just be :rename {default foo}

dominicm20:04:59

Remind me why we need sugar?

dnolen20:04:49

because the :rename trick brings in an object not a namespace thing

dnolen20:04:03

foo$default :as foo would support foo/api

dnolen20:04:20

and you could use it in combination with [foo :as f]

dominicm20:04:04

I'm a little out of touch with esm, if you do import x from "y" does x.z make sense for all things?

dnolen20:04:06

this about about using JS libs as namespaces

dominicm20:04:32

I'm assuming default could be a string.

dnolen20:04:41

there's another interesting idea here

thheller20:04:56

Its usually a single object like a class or so. haven't seen a case where that be an actual "namespace"

4
dnolen20:04:19

which I floated around before which is that foo$default might not need to be specific to ES6

dnolen20:04:30

another reason I don't really like :default that much

thheller20:04:30

only in the cases where CommonJS is actually imported that way .. so import React from "react" and React.createElement

dnolen20:04:44

someGlobal$bar would be possibility

dnolen20:04:59

i.e. some JS object on the page that's global

dnolen21:04:09

and you could get externs infer for that too

dnolen21:04:24

rather than naked js/...

dnolen21:04:10

so that's another to weigh, Foo$Bar could be generic pattern rather hard coded to default problem

thheller21:04:14

@dnolen not sure what you are talking about

lilactown21:04:48

so you could do (:require [window$AbortContoller :as AbortContoller]) ?

lilactown21:04:56

what about using :import?

dnolen21:04:27

@lilactown yes all these standard apis could be required as namespaces

dnolen21:04:31

it would cleanup usage

dnolen21:04:00

@lilactown (:import ...) is really just a hack for GCL

dnolen21:04:11

because the namespace can be an object which is really annoying

dnolen21:04:56

@lilactown I don't think Foo$Bar pattern can help much with that

dnolen21:04:53

anyways I'm warming up to Foo$Bar because it's more general, and default falls out of it, instead of being designed for it

dnolen21:04:23

wanting to treat some property as a namespace is a generally useful thing

lilactown21:04:29

how will it work with imports that are strings? "@corp/my-lib$default" seems gross to my eyes

dnolen21:04:18

would be exactly the same, and again not specific to default

dnolen21:04:36

you could get at anything in that lib as a namespace

dnolen21:04:53

introducing novelty to the delimiter here is not that interesting to me

dnolen21:04:00

Clojure already has a pattern we already use it

lilactown21:04:03

and I guess, IMO it would be better for external analysis to not have to parse strings / symbols

thheller21:04:04

so we are not modifiying the ns because :default is "new" but we are adding a whole bunch of magic for accessing nested properties?

dnolen21:04:18

"whole bunch of magic"

dnolen21:04:38

this isn't a very challenging change we already have everything to return whatever from a resolve

thheller21:04:16

I mean adding new functionality to ns. not talking about implementation at all, I know that is trivial.

dnolen21:04:38

it's really a change to resolve

dnolen21:04:40

barely a change to ns

lilactown21:04:18

yeah I think the semantics of a keyword vs a special string/symbol are not really different to me as a consumer, which I think is what thheller means

thheller21:04:35

I think regular JS interop is fine for this don't know why the ns/resolve would need gain that functionality

thheller21:04:48

don't even want to think about an npm package that has an actual $ in its name. I'm sure there exists one πŸ˜›

dnolen21:04:48

@lilactown I'm already talking about bigger problem now

dnolen21:04:59

the discussion was just think about the default problem

dnolen21:04:10

but now maybe a more general issue to solve

dnolen21:04:42

the default issue is the same as many other things you can't practically require

dnolen21:04:52

that requires going to globals or accessing objects

lilactown21:04:17

I am tracking your thoughts I think. I disagree with the ergonomics of using a special $ delimiter

thheller21:04:17

@dnolen just so I understand what you are proposing. suppose there is a npm package that exports something nested like react-native :as rn rn/AppRegistry.registerComponent. you want (:require [react-native$AppRegistry :as reg]) reg/registerComponent?

thheller21:04:21

I don't hate the idea but this definitely falls into the magic category for me.

thheller21:04:58

scratch that ... I don't like magic characters.

lilactown21:04:41

(ns 
  ;; add a new `:with` keyword that provides the behavior
  ;; to use a JS property as an ns, provide externs, etc.:
  (:require ["@corp/my-lib" :as my-lib :with [default :as my-lib-default]]
            [window :with [AbortController]]))

lilactown21:04:26

the difference between :with and :refer / :rename is that the symbol would be treated as a namespace, not a value

thheller21:04:28

@lilactown noooooooooooooooo πŸ˜›

dnolen21:04:21

@thheller this is no longer about just node_modules

dnolen21:04:36

it does not matter what require you are using, it would work

dnolen21:04:01

there's nothing magical about it, it's just a custom resolve

dnolen21:04:07

it's almost identical to inner classes in Clojure

dnolen21:04:16

there something in a package that you want to refer to, but it's impractical

dnolen21:04:55

in this case there some property in JS which is effectively a namespace

dnolen21:04:12

i.e. a global, i.e. a property, i.e default, it doesn't not matter

dnolen21:04:20

it would be better to use that thing as a namespace from ClojureScript

dnolen21:04:54

so less interested in now in feedback about default problem

dnolen21:04:12

more interested in feedback about a problem in literally all ClojureScript codebases right now

dnolen21:04:48

tons of libraries / apps right now accessing properties which are just namespaces, global which are just namespaces

thheller21:04:54

I'm not asking about node_modules. I used that as an example of how the actual syntax is supposed to look

dnolen21:04:05

it would only work on the ns identifier itself nowhere else, and probably the additional constraint of only one level

dominicm21:04:28

I'm a browser guy, so I'm thinking of something like js/window.location.href, is that the kind of thing you'd like an alternative to?

dnolen21:04:28

[FooGlobal$AnalyticsThingy ...]

dnolen21:04:42

JS globals is also possible

dnolen21:04:46

[Math ...]

dnolen21:04:04

I won't cry for js/Math or losing the special casing

thheller21:04:18

an how do you tell that FooGlobal is an actual global? I mean is every symbol now valid?

thheller21:04:11

[FooGolbal$AnalyticsThingy ...]? how is the user warned about that typo?

dnolen21:04:31

for known HTML stuff, Closure can tell us - foreign libs w/o :file is an open JIRA thing

dnolen21:04:50

the latter give you a way to say you want to use something from somewhere not in the build

lilactown21:04:53

1. This is sort of moving the goal posts, but I appreciate thinking more generally about this problem 2. I hate the $ as a special delimiter inside of a symbol or string. We have EDN at our disposal; we should be able to craft syntax that represents this

nwjsmith21:04:33

$ has prior art though...

thheller21:04:00

prior art that is abused for some entirely different purpose

thheller21:04:30

$ is just how the JVM handles inner class names so its more a JVM thing than it is a clojure thing IMHO

πŸ‘ 8
nwjsmith21:04:55

But it's how you refer to an inner class, which rhymes with what you want to do here

dnolen21:04:12

also $ is ugly

dnolen21:04:17

so less likely to clash w/ anything

dnolen21:04:17

but if people have way better ideas for the delimiter then tell me

dnolen21:04:35

no ideas about extending the ns form please

thheller21:04:53

I don't like the idea simply because of the magic character. it does not matter which character it is.

dominicm21:04:14

Given that . Works as the delimiter already, why something new?

dominicm21:04:12

"." Is probably a good choice actually... Nobody will use that in js

thheller21:04:23

. is definitely valid and used in a few npm package names so would lead to ambiguities when trying to figure out which package name the user meant

lilactown21:04:59

my assumption was that it would make it difficult to resolve a.b.c where b or c could be a namespace or property

dominicm21:04:16

Really!? Gah. I thought it might particularly not be valid, due to conflict with properties.

thheller21:04:19

https://www.npmjs.com/package/object.assign and of course there is a object package as well

lilactown21:04:22

I think it’s better to extend the ns form parsing than to add a delimiter. Β―\(ツ)/Β―

Oliver George22:04:24

How about the $ and a reader macro which allows a more familiar require string for node modules to get munged into an unambiguous form

dnolen22:04:18

@olivergeorge what do you mean, though reader macro is almost automatic no

Oliver George22:04:54

Rough thought was something like this

(:require [#npm "some/mad.convention"])
or if needed going a step higher to allow for richer syntax
(:require #npm ["@corp/my-lib" :as my-lib :with [default :as my-lib-default]])

Oliver George22:04:21

fair enough πŸ™‚

Oliver George22:04:23

The conflicting syntax & conventions does complicate finding a compatible approach without having "modes" of interpretation... that lead me to a munging wrapper.

Oliver George22:04:38

I'm not that offended by $ becoming more common

borkdude22:04:18

> adding more stuff to the ns form that isn't Clojure stuff, that's the hard design constraint I'm not sure if I understand this. trying to fit all the npm lib require stuff into :require ... why not something more dedicated like :npm-require, less confusion overall for tooling as well maybe

thheller22:04:44

this isn't just about npm. IMHO a string is already clear enough in that the require refers to JS code and not a CLJS namespace

thheller22:04:18

shadow-cljs had (:js-require ["foo" ...]) for a while but rolling that into :require was way nicer

borkdude22:04:58

are string require names obligated for npm libs btw? I've had one example in #clj-kondo where a user didn't use a string for an npm lib and this resulted in some warnings, because clj-kondo expects you do use a string for npm libs

thheller22:04:52

CLJS allows any symbol if there is a folder of that name in node_modules. so (:require [react]) is valid if it indexed a node_modules/react before.

thheller22:04:48

shadow-cljs also allows (:require ["./foo.js" :asfoo]) for directly accessing js files from the classpath, so not npm.

thheller22:04:21

but thats shadow-cljs only and not supported in CLJS (and was rejected there too)

borkdude22:04:30

would it be reasonable to ask users to use strings to indicate js libs?

thheller22:04:14

I think so yes. I do recommend that. I hate that node_modules/anything automatically becomes a valid symbol to require

thheller22:04:25

but that ship has sailed and is not going to change

thheller22:04:46

in shadow-cljs it is not no. in CLJS you can actually do (:require ["clojure.string" :as str]) I believe which I would consider a bug πŸ˜‰

dpsutton22:04:40

clojure-lsp does not like the string requires at the moment

borkdude22:04:13

(ns foo
  (:require
   ["markdown-it" :as md]
   ["markdown-it-texmath" :as md-texmath]
   [com.nextjournal.editor.markdown.todo-lists :as todo-lists]
   [com.nextjournal.editor.markdown.gh-preamble :as gh-preamble]))

(def mdit (.. (md #js{:linkify true :breaks false :html true})
              (use md-texmath)
              (use gh-preamble/Plugin)
              (use todo-lists/Plugin)))
This was the example, but without the string quotes in the requires. This confused clj-kondo because it doesn't expect the alias to be usable as an object, when it's not a JS lib

dnolen22:04:27

@borkdude strings are just allowed in general, there's no interesting reason to be more restrictive, use to indicate anything is not a good idea

dnolen22:04:52

the real problem which only tangentially has to do w/ JS is that Clojure notion of symbols is related to what works for Java packages

borkdude22:04:05

so how can tooling "see" if an alias is a js object or just a CLJS alias?

dnolen22:04:12

strings widen what characters are allowed in an effective package name

dnolen22:04:36

what shadow-cljs does here is just not up for discussing let's stop talking about this in this channel

dnolen22:04:48

nothing is going to change here

borkdude22:04:04

I accept that πŸ™‚ But is there a way to distinguish between a JS obj or alias from a tooling perspective (without inspecting the thing at runtime)?

borkdude22:04:48

if the answer is no, then clj-kondo should treat all aliases as potential js objects

dnolen22:04:53

@borkdude I think what's happening there is the invokeable ns stuff since some JS libs do that, what's return is a function you have to invoke

dnolen22:04:50

@borkdude so I think you can't reasonably lint that, we don't do any kind of detection either really

dnolen23:04:00

(ns foo
  (:require ["@foo/bar$default" :as foo.bar]))

(ns foo
  (:require [global$console :as console]))

(ns foo
  (:require [global$document :as doc]))

(ns foo
  (:require [global$crypto :as crypto :refer [subtle]]))

(ns foo
  (:require [global$CustomModule :refer [bar]]))

dnolen23:04:24

some typical cases I've observed in pretty much every ClojureScript project that I've worked on

dnolen23:04:42

the global$foo would resolve to goog.global and be validated against the existing externs (when available)

dnolen23:04:23

towards the end it's also quite common in a custom JS context (React Native obviously but anything, Ambly, raw JSCore) that you installed a native module at the global level

dnolen23:04:18

so there's a lot of global patterns here that can be lifted into the ns form

dnolen23:04:41

in truth that's what the global is, effectively a package, but we don't treat it as such

dnolen23:04:24

it should in fact probably just be goog.global to avoid a little bit of special handling

dnolen23:04:44

goog.global$crypto

dnolen23:04:14

the default stuff is just extra

dnolen23:04:58

goog.global$Math etc. would provide a discplined way to handle the standard JS builtins without the current hardcoding in the analyzer

dnolen23:04:12

I suppose for looks in this case goog.global.Math is fine

dnolen23:04:17

hrm what about : ?

dnolen23:04:59

(require ["@foo/bar:default"]) and (require foo.bar:default) ?

dnolen23:04:39

though symbol w/ : is a perfectly valid ns name in Clojure, where $ is likely ugly enough to be uncommon

lilactown23:04:51

I like it slightly more than $. it feels more aesthetic

dnolen23:04:05

problem is it's pretty valid

lilactown23:04:08

I would be super surprised to see either a : or $ in a Clojure ns

dnolen23:04:27

$ less likely because it appears when munging

dnolen23:04:43

anything else I wouldn't place any bets

Oliver George23:04:38

A string form with a space?

lilactown23:04:46

I wish we could use / but that’s even more likely to show up in js module import

dnolen23:04:52

yeah no way

dnolen23:04:50

ok gonna take break, please ruminate on this over the weekend if you're feeling invested πŸ™‚

πŸ‘ 4
richiardiandrea23:04:27

read the whole thread and I have to say I like the whole idea personally so I give my πŸ‘ I even like the $ cause it is consistent with Clojure

πŸ‘ 4
dnolen23:04:11

thanks for the feedback!