Fork me on GitHub
#cljs-dev
<
2017-02-01
>
juhoteperi13:02:51

WARNING: Use of undeclared Var schema.core/RegExp at line 356 /home/juho/.boot/cache/tmp/home/juho/Source/***/***/4i8/-uc8bn7/js/main.out/schema/core.cljs
WARNING: Use of undeclared Var bidi.bidi/RegExp at line 232 /home/juho/.boot/cache/tmp/home/juho/Source/***/***/4i8/-uc8bn7/js/main.out/bidi/bidi.cljc

juhoteperi13:02:06

This was with :advanced build

juhoteperi13:02:48

And second run also reproduced this

juhoteperi13:02:49

I'll hope this is consistent and try bisecting the cause

juhoteperi13:02:11

makes sense, as the error from Schema is related to extend-protocol

dnolen15:02:14

@juhoteperi that does seem like a Schema bug to me

dnolen15:02:25

I don’t see how propagating type hints should cause problems

dnolen15:02:42

I mean I can be convinced otherwise if we have some example that isn’t Schema related

dnolen15:02:43

@juhoteperi linking to the source code of bidi and schema here would be useful.

dnolen15:02:57

(sorry I see now that Bidi also has this problem, I missed that)

juhoteperi15:02:41

interestingly bidi has another extend-protocol related to js/RegExp that doesn't cause warning: https://github.com/juxt/bidi/blob/master/src/bidi/bidi.cljc#L99

dnolen15:02:42

@juhoteperi huh - ok so that seems we can easily make a minimal case

dnolen15:02:53

just (extend-prototocol js/RegExp …)

dnolen15:02:39

or at least experiment with what might trigger

dnolen15:02:50

I do see that in the cases about the argument count differs

juhoteperi15:02:04

in Schema case all methods take 1 arg, in bidi 2

juhoteperi15:02:13

Also, I can still only reproduce this when doing :advanced build

juhoteperi15:02:06

Didn't try making standalone test project yet, but I tried to make test case to cljs.compiler-tests, but didn't succeed reproducing problem there

dnolen15:02:22

@juhoteperi I think under advanced there may be cases where we try to resolve type hints

dnolen15:02:29

I’m pretty sure I did that somewhere at some point

juhoteperi15:02:32

Ah, :static-fns true seems to cause this!

juhoteperi15:02:15

that defaults to true with :advanced

juhoteperi15:02:26

(let [cenv (env/default-compiler-env {:static-fns true})]
    (binding [ana/*unchecked-if* false]
      (capture-warnings
        (env/with-compiler-env cenv
          (comp/emit
            (ana/analyze
              aenv
              '(do (defprotocol Foo
                     (spec [this]))
                   (extend-protocol Foo
                     js/RegExp
                     (spec [this] true)))))))))
Should something like this work? Currently I see lots of warnings: WARNING: Use of undeclared Var cljs.core/not at line 229

dnolen15:02:50

@juhoteperi you need to analyze with cljs.compiler/with-core macro

dnolen15:02:58

so you get all the core defs

juhoteperi15:02:44

Okay. No warnings now.

thheller15:02:02

I'v been hunting a similar issue

thheller15:02:30

not any closer yet but I think it is related to multiple uses of extend-protocol

thheller15:02:24

(defprotocol X
  (x [x]))

(defprotocol Y
  (y [y]))

(extend-protocol X
  js/RegExp
  (x [x]
    (y x)))

(extend-protocol Y
  js/RegExp
  (y [y]
    :y))

thheller15:02:55

warning on line with (y x)

thheller15:02:22

but is should be the minimal case

thheller15:02:13

(defprotocol X
  (x [x]))

(defprotocol Y
  (y [y]))

(extend-type js/RegExp
  X
  (x [this]
    (y this)) ;; << warning here

  Y
  (y [this]
    :y))

thheller15:02:18

maybe this is shorter

juhoteperi15:02:39

Yep, I can reproduce the warning with that code on real project

thheller15:02:49

produces no warning when using a normal deftype

juhoteperi15:02:01

[[:undeclared-var "WARNING: Use of undeclared Var cljs.user/RegExp at line 244 "]]

juhoteperi15:02:14

Now I got the test case to generate the warning

juhoteperi16:02:27

And attached patch with a test case

juhoteperi16:02:12

which is broken.. oops (fixed now)

dnolen16:02:15

@anmonteiro merged your package.json patch - thanks

anmonteiro16:02:49

👍 awesome

anmonteiro16:02:16

We should make a note to update the guide when the next version is released

anmonteiro16:02:30

Since JSONStream is no longer required

dnolen17:02:00

yeah I’m shooting for bugfix release on Friday

anmonteiro17:02:09

I'm fixing CLJS-1925 now

anmonteiro17:02:30

it's the consequence of something I didn't fix correctly a while ago

juhoteperi18:02:28

@anmonteiro When you mentioned using React from Npm, did you refer to node_modules/react/react.js or node_modules/react/dist/react.js

juhoteperi18:02:51

Hmm, the JavaScript modules guide at least uses just require("react")

juhoteperi18:02:50

I'm testing packaging npm files as jars

juhoteperi18:02:46

ah, the problem is that I eagerly package fbjs and other dependencies, that are not really needed for browser use

juhoteperi18:02:15

Currently process-js-modules tries to process all the files defined in foreign-libs

juhoteperi18:02:35

I wonder if it would be possible to only process required files

juhoteperi18:02:12

Now I need to be careful to only package files that can be processed by Closure

dnolen18:02:29

@juhoteperi I’m pretty sure it doesn’t do that? I thought only if :module-type is set

juhoteperi18:02:07

Yes, I meant foreign-libs with module-type set

dnolen18:02:50

right this is why we use an entry point for node_modules support

dnolen18:02:54

to avoid that problem

dnolen18:02:36

another possibility is looking into dependency_mode=STRICT feature of Closure Compiler

dnolen18:02:46

maybe that just works? I haven’t tried it

anmonteiro21:02:11

@dnolen the issue was caused by an improper fix to CLJS-1607 (previous patch of mine) which is now fixed in the patch I just attached