Fork me on GitHub
#shadow-cljs
<
2018-05-29
>
axrs05:05:18

Has anyone noticed an error when releasing for :browser with cljs.spec.alpha? We've just started to see and error with the compiled modules with: - shadow-cljs 2.3.30 - clojurescript 1.10.238 - node 10.2.1

Uncaught TypeError: L.l is not a function
at dist/cljs/spec/alpha.cljs:91:19 <- dist/shared.js:513:307

TypeError: L.l is not a function
    at $n (dist/cljs/spec/alpha.cljs:91:19 <- dist/shared.js:513:307)
    at no (dist/cljs/spec/alpha.cljs:300:33 <- dist/shared.js:519:141)
    at dist/io/comp/spec/user_spec.cljc:7:0 
The line in question is: (s/def :user/first-name u/non-empty-string-conformer), which passes all node, doo and clojure tests

thheller05:05:24

@axrs try compiling with shadow-cljs release app --pseudo-names or --debug

axrs05:05:07

Thanks @thheller, I'll give it a shot and report back

thheller05:05:50

generally that looks like externs trouble but might just be a spec implementation error

thheller05:05:30

I think I ran into something similar when builing a custom spec with deftype and not defrecord

thheller05:05:52

dunno what u/non-empty-string-conformer is

axrs05:05:47

u/non-empty-string-conformer is just a function

axrs05:05:29

doesn't really do too much that would through it off unfortunately

axrs05:05:34

(defn- non-empty-string [s]
  (if (or (not (string? s))
          (string/blank? s))
    ::s/invalid
    (string/trim s)))

(def non-empty-string-conformer (s/conformer non-empty-string))

axrs05:05:37

Thanks @thheller, that solved the problem! Was a strange one

thheller05:05:00

what solved the problem?

thheller05:05:46

don't actually use the --pseudo-names output. that is just for debugging purposes.

thheller05:05:06

if the problem goes away by doing so you have externs problems

thheller05:05:10

try shadow-cljs check app

axrs06:05:50

Ahhh. Yeah, --pseudo-names solved the problem.

thheller06:05:01

yes but it also makes your builds huge

axrs06:05:09

I'll run a check and review all of our interop.

thheller06:05:43

externs problems are sometimes weird to debug since they can mess with your code in unexpected ways

axrs06:05:12

Thanks. The check shows we have a few places we can fix up. I'll see if inferring helps shortly

axrs06:05:09

Maybe found another problem now after running check IllegalArgumentException: No matching method found: getGlobalType for class com.google.javascript.rhino.jstype.JSTypeRegistry

axrs06:05:21

IllegalArgumentException: No matching method found: getGlobalType for class com.google.javascript.rhino.jstype.JSTypeRegistry
	clojure.lang.Reflector.invokeMatchingMethod (Reflector.java:53)
	clojure.lang.Reflector.invokeInstanceMethod (Reflector.java:28)
	shadow.build.closure/register-cljs-protocol-properties (closure.clj:387)
	shadow.build.closure/register-cljs-protocol-properties (closure.clj:372)
	shadow.build.closure/check/fn--13533 (closure.clj:1078)
	shadow.build.closure/check (closure.clj:1073)
	shadow.build.closure/check (closure.clj:1071)
	shadow.build/check (build.clj:371)

thheller06:05:44

hmm which version are you running? I fixed a while ago

thheller06:05:00

maybe your closure version is out of date?

axrs06:05:37

shadow-cljs - config: ... cli version: 2.3.30 node: v10.2.1, Clojure 1.9.0, lein 2.8.1

thheller06:05:08

run lein deps :tree

thheller06:05:17

check which closure-compiler-unshaded version you get

axrs06:05:24

My problem child... lein...

thheller06:05:30

should be [com.google.javascript/closure-compiler-unshaded "v20180506"]

axrs06:05:49

My unshaded is coming from clojurescript 1.10.238

axrs06:05:03

I'll add a dep and try again

thheller06:05:05

try

[org.clojure/clojurescript "1.10.238"
    :exclusions
    [com.google.javascript/closure-compiler-unshaded]]

thheller06:05:28

or just remove clojurescript dep entirely. shadow-cljs will bring that in

axrs06:05:11

Yep, excluding the unshaded fixed the getGlobalType error. I've got a few warnings that may be the underlying cause (externs related). I'll let you know how I go 🙂

axrs06:05:19

Thanks for everything @thheller

thheller06:05:56

check may be reporting a few false positives. it is not perfect. happy to help if you have questions

axrs07:05:11

@thheller unfortunately, the issue still remains. I've fixed up all the externs within our source. The only thing left I can think of is the (s/def is doing something else funky

thheller07:05:38

do you have any other JS running in your page?

thheller07:05:47

are you using multiple :modules?

axrs07:05:05

No to other JS (in this scenario), but yes to modules

thheller07:05:39

absolutely no other JS in the page?

axrs07:05:07

Yep. Created a seperate html file and loaded just those in

thheller07:05:57

try :compiler-options {:output-wrapper true} just to be sure (in your build config)

axrs07:05:47

Thanks. Will try 🙂

thheller07:05:17

it can sometimes be tough to debug those issues, might need to open up the debugger and step through the code to find out exactly whats happening

axrs07:05:15

Yeah. It's going to be tricky to do so I think. loading all scripts into the page causes it to hang while it trys to do it's thing. I should be able to debug one script at a time hopefully

thheller07:05:36

the problem is L.l is not a function which is closure-shortened variable. with :pseudo-names it will not be that short by rather $something$.$fooBar$

thheller07:05:54

so the changes of that conflicting with something are slim over just .l conflicting with something

thheller07:05:23

you can try adding a externs/<your-build>.txt and adding one line with just l

thheller07:05:46

but it would still be worth finding out where the l is coming from in the first place

axrs07:05:57

Yep. That all makes sense and confirms what I thought it was doing.

thheller07:05:56

curious if the .txt file fixes it or if it just becomes another character 😛

axrs08:05:00

The output wrapper actually fixed it

thheller08:05:16

so then there must be some external JS in your page

axrs08:05:26

I'm thinking maybe a race condition

axrs08:05:39

shared.js and app.js were being loaded and evaluated at the same time

thheller08:05:45

or some JS that just forcefully sets L maybe?

thheller08:05:52

no that can't happen

thheller08:05:46

JS engines are single threaded. so "at the same time" is not possible

axrs08:05:13

That's true.

thheller08:05:28

maybe something just sets window.L or so

axrs08:05:07

Hmm. Maybe. We do have a few npm dependencies that are being being used with Shadow. One of them could be doing something internally.

thheller08:05:07

are you using mapbox? I think they use L by default?

thheller08:05:20

or rather leaflet?

thheller08:05:08

in your page with the error check what L is in the console

thheller08:05:54

I assumed the .l property was the problem but maybe its just the L global

thheller08:05:20

but :output-wrapper is fine to use as well

thheller08:05:30

just makes the build slightly larger

axrs08:05:55

Yep. We leaflet is used as a sub dependency in a package, as an npm dep. So that might be the culprit.

axrs08:05:20

Which would make sense, as it's loaded after the shared modules

axrs08:05:06

Thanks for the help again. Such a simple thing can cause a big issue...

thheller08:05:56

yep confirmed. leaflet always creates the L global

thheller08:05:54

guess thats on purpose

axrs08:05:54

Yeah. I think I'll be killing off leaflet. It's being a problem child

thheller08:05:34

I'm reserving L by default now so the compiler will never attempt to use it

thheller08:05:45

just in case someone uses leaflet. not a problem for builds that don't.

axrs08:05:42

Thanks. It feels wrong to do so, but will help others out in the future no doubt

thheller08:05:56

I might just default :output-wrapper to true as well. its not currently default when using multiple modules because the build gets slightly larger

thheller08:05:07

but would be the safer default

axrs08:05:52

Cool. It might be worth adding it to the docs as well. It's not listed as a supported option, but is mentioned above somewhere.

tianshu09:05:18

@thheller now shadow-cljs + lein + cider works great!

nijk10:05:38

Does anyone know if it’s possible to use the suppress annotations provided by closure compiler? Or if not, any kind of workaround that would allow something like the exclusion of particular files from throwing warnings? https://github.com/google/closure-compiler/wiki/@suppress-annotations

thheller10:05:30

@nick828 which warnings do you get?

nijk10:05:54

Use of undeclared Var

thheller10:05:30

how it that related to an annotation?

nijk10:05:52

Well my understanding is that it will allow me to do this: > If you cannot easily fix a JsCompiler warning, you can suppress it using the @suppress annotation in the function’s JSDoc block

nijk10:05:27

I would prefer to fix the warning, but at this stage I’m looking at my options

thheller10:05:00

what kind of code are we looking at?

thheller10:05:32

and what is the full warning? the annotations really only suppress warnings for the function you annotate and that should never be necessary in CLJS

nijk10:05:33

It’s cljs and I’m implementing code-splitting - the warning is triggered by the fact that I need to refer to a function in another namespace that has not been required. If I were to require it in then I believe that the bundles would not be correct. Perhaps this would be better dealt with some other way (by using a macro?)

thheller10:05:07

so the "official" solution is to use resolve

nijk10:05:21

oooo, ok - please do tell me more

thheller10:05:31

so (let [something (resolve 'some.ns/foo)] (something))

thheller10:05:43

that way your code can call a function that it has no require for

nijk10:05:15

sounds great, thanks @thheller

thheller10:05:44

note that you can only actually call the resolved var after the code has been loaded. otherwise it'll just error out

nijk10:05:15

Timely message! I’ve just noticed that 🙂

nijk10:05:20

@thheller Thanks for your help there, it’s now working beautifully 🙂 I’m not sure if I’ve struggled with this because I’m new to Clojure & ClojureScript but I wonder if a slightly fuller example of a code-splitting implementation might be helpful to others? I’ve been following this: https://shadow-cljs.github.io/docs/UsersGuide.html#_loading_code_dynamically

thheller11:05:28

@nick828 unfortunately pretty much all uses for code-splitting and dynamic loading rely pretty heavily on the framework you are using and how your code it structured

thheller11:05:39

coming up with a general purpose example is pretty tough

thheller11:05:45

;; foo.bar is part of module :foo
(def some-var (resolve 'foo.bar/x))

(-> (loader/load "foo")
    (.then (fn []
             ;; foo.bar is now loaded
             (some-var))))

thheller11:05:59

so basically whenever you want to call some-var you probably need to wrap it in a load call to ensure it is loaded

thheller11:05:16

you call check (loader/loaded? "foo") as well

thheller11:05:04

using multi-methods also helps

thheller11:05:09

typically you'd trigger the load in the router or on click

thheller11:05:27

are there any reagent/re-frame examples out there that use routing?

nijk12:05:08

@thheller fair point, I guess the resolve call was all I needed to understand in the context of loading modules dynamically. Good point on checking that the module is loaded, I had noticed that from the docs. For what it’s worth I’m using Kee-frame for routing although there are many ways to handle the routing, that said I wasn’t suggesting that the docs had anything more than a very basic example of using resolve - agnostic of any routing…

thheller12:05:00

is there a kee-frame template or so?

nijk12:05:59

A Kee-frame template that gives an example of how code-split modules could be loaded?

thheller12:05:32

only the routing part

nijk12:05:36

I’m not sure I quite understand what you’ve got in mind, there is a demo project linked to from Kee-frame’s readme: https://github.com/ingesolvoll/kee-frame-sample, the routers.cljs for the demo project is here: https://github.com/ingesolvoll/kee-frame-sample/blob/master/src/cljs/kee_frame_sample/routers.cljs

thheller12:05:19

not a clue what that is about. why 3 different routers?

thheller12:05:58

looks like it already pretty heavily abstracts all the routing logic though. might make it harder to adapt it to be aware of splits

nijk13:05:55

The commit message might be helpful to explain that: > Changed API to support different routing libs. Example implementation…

nijk13:05:25

Do you just want a boilerplate for handling routes in Kee-frame so that you can give an example implementation of code-splitting in your docs?

thheller13:05:19

not the docs but this example could be made more useful. https://github.com/shadow-cljs/examples/tree/master/code-split

thheller13:05:56

I can probably do a framework-less generic example

nijk13:05:11

Ah right! I’m with you now - yeah I’d recommend steering clear of a framework for this, I wonder if a very basic bidi implementation would be enough - you might need more of your own boilerplate to make it work though

nijk13:05:53

to be clear, I’m not saying that this is absolutely necessary in order to understand how to implement code-splitting using Shadow-cljs - just that the current documentation could be a bit clearer about good practices on resolving namespaces that may not have been loaded at the point where the code is first executed

thheller13:05:46

yeah. it is tricky to do this correctly so the docs should definitely clarify this a lot more

nijk13:05:12

I’d love to see an example of how you have done this correctly 🙂 that would be super useful to me

thheller13:05:27

there could also be a little more support by a macro or two

thheller13:05:07

currently you have to know which module a namespace is in which is a bit annoying

nijk13:05:40

Yes, that’s where I think my current implementation is not so great

nijk13:05:03

Are you familiar with ReactBoilerplate?

thheller13:05:33

looked at it yes, used it no

nijk13:05:21

it might be worth having a look at the way it handles code splitting - I haven’t used it for 6months+ but from memory it was pretty straight-forward to implement

nijk13:05:09

It’ll obviously be rather different from how things are handled in the cljs world but I think a lot of the underlying patterns are quite similar

thheller13:05:17

main problem for me is that I don't use reagent. so I have no idea how you'd write it in reagent

nijk13:05:51

Ok, I might be able to convince my people to allow me spin off a generic repo to provide an example in Reagent/Reframe

thheller13:05:30

(defn async-component [the-module the-var]
  (let [loading-ref (r/atom false)]
    (-> (fn []
          (if loading-ref
            [:div "Loading ..."]
            (the-var)))
        (with-meta
          {:component-did-mount
           (fn [this]
             (-> (loader/load the-module)
                 (.then #(reset! loading-ref true))))}))))

thheller13:05:02

(async-component "foo" (resolve 'some.foo/var))

thheller13:05:37

assuming (ns some.foo) (defn var [] [:div "hello world"])

thheller13:05:09

not exactly sure how you are supposed to do lifecycles in reagent

thheller13:05:28

so instead of [live/live] you do the async-component call

nijk13:05:26

This is similar to my first attempt at it, I’ve now moved to a reframe approach using event handlers (coeffects/effects/interceptors) for loading the module at the right time in the application

thheller13:05:05

basically you want a stateful component that re-renders when the code is loaded

thheller13:05:26

alternative when using something like re-frame that has an event handler like :set-current-page or so

thheller13:05:39

instead of directly setting the page you figure out which module to load

thheller13:05:58

and then do the actual :set-current-page in a callback

nijk14:05:11

Not sure if you saw my message above: > I’ve now moved to a reframe approach using event handlers (coeffects/effects/interceptors) for loading the module at the right time in the application

nijk14:05:58

From the reading I’ve done on approaching problems with a React vs Reframe mindset, I’ve decided to move away from the stateful component pattern

thheller14:05:08

doesn't matter which approach you use. somewhere you need some state that gets updated when the code is loaded and triggers a re-render

thheller14:05:22

does not matter *where* the state is

nijk14:05:24

Agreed, the pattern you are suggesting is what needs to happen

nijk14:05:41

The ‘how’ is a little less important

justinlee15:05:53

@thheller is this the bhauman library for checking maps you were talking about the other day? https://github.com/bhauman/spell-spec it looks amazing

thheller15:05:32

figwheel had something similar. looks like he extracted it into a proper lib. nice.

thheller15:05:45

but yeah I was talking about those kinds of error messages. figwheel has some really good error messages.

theeternalpulse18:05:53

is cljss an offshoot of the shadow.css stuff in the wiki

theeternalpulse18:05:04

I noticed that it has similar macro names

thheller18:05:56

no. I think we more or less both took styled-components from the react work as inspiration

thheller18:05:14

the implementations are entirely different

wilkerlucio23:05:41

@thheller do you we could update the manifest.json automatically after changing the manifest.edn?