Fork me on GitHub
#cljs-dev
<
2018-04-17
>
denisj02:04:40

Existing code using

(cljs.reader/register-tag-parser! ...)
works with Clojure 1.8.0 + Clojurescript 1.9.671 but breaks with Clojurescript 1.10.238 Compiles but then runtime unit tests fail with "No reader function for tag xx" Can someone please point me at how this works with later cljs 1.10.x

r0man11:04:30

@dnolen Hi David, I think I got something working. I attached a patch to https://dev.clojure.org/jira/browse/CLJS-2682 . I basically added the information from the loader compilation pass to the compiler state, so it is available when the cljs.loader namespace gets compiled again (from the REPL for example). I also excluded the cljs.loader namespace from the :aot-cache again, since at the point the cache is written we don't have module-infos and module-uris available yet. Another option would be to populate the cache for the cljs.loader namespace in the compile-loader function. I thought about calling compile-from-jar from there, but at this point I didn't have access to the jar-file anymore. Let me know what you think.

mfikes11:04:30

@denisj If you don't get any response, I'd at least file a JIRA with a minimal repro. (That would be a good thing to do if something regressed between ClojureScript 1.9.671 and 1.10.238.) With a minimal repro, we could more readily sort out if it is an upstream problem (in the reader), or in the compiler itself.

denisj08:04:10

Thanks to @U05224H0W, he pointed out expectation is for registered tag should be symbol. Even though plain string tags worked previously, after changing to symbols our tests are passing again and backward compatible.

dnolen14:04:17

@denisj thanks, but yes, closed as that is no longer supported

👍 4
dnolen12:04:30

yes @denisj file an issue - we have tests for reader literals but I guess you’re doing something that we missed or don’t check

denisj13:04:24

Thanks @mfikes & @dnolen I will, just wanted to make sure I wasn't missing something obvious

juhoteperi14:04:33

@mfikes you said on Figwheel PR that Node modules indexing is unnecessary for React-native users, but I think it can be useful for that environment also, with :target :nodejs indexing node_modules allows using (:require [react-native :as rn]) and Cljs will generate js/require call (which RN packager can read).

juhoteperi14:04:11

If indexing can take up to 45seconds, we should probably look into optimizing it

mfikes14:04:21

I also agree on that. 🙂

mfikes14:04:45

I’ll write a JIRA to capture a perf enhancement.

juhoteperi14:04:22

I think for :nodejs we only call index-node-modules-dir (which checks the directory tree using JVM) not index-node-modules (which reads dependency tree using Node JS)

juhoteperi14:04:58

Ah no, it goes node-inputs -> node-modules-deps -> module_deps.js

juhoteperi14:04:21

Can you quickly check (time (index-node-modules-dir)) and maube (time (index-node-modules ["react-native"])), to give some idea which part is taking long

mfikes14:04:45

@juhoteperi Unfortunately, without digging deeper, no useful information on the second one: https://gist.github.com/mfikes/c99cfac7b9f5ae48fc9644bbde492a3c

juhoteperi14:04:48

Ah yes, you need to run npm install @cljs-oss/module-deps for Cljs module indexing to work. But we can already see that the indexing we do on JVM side is quite slow.

juhoteperi14:04:07

Okay. Looks like some lib used by RN is not supported yet.

Alex Miller (Clojure team)15:04:40

You can throw it in a comment on 73

juhoteperi15:04:13

@lee.justin.m No, Jira uses it's own archaic formatting syntax, e.g. {{inline-monospace}} and {code} {/code}

justinlee15:04:48

thanks. i should have read the help bubble there.

juhoteperi15:04:51

Or more specifically the Jira version used by Clojure doesn't support Markdown but Jira has supported for years, at least using a plugin

richiardiandrea15:04:39

Talking about indexing, does the compiler add entries for fs, path and all the built in node modules? Looking at https://dev.clojure.org/jira/browse/CLJS-2737

juhoteperi16:04:02

Files for built-in Node files are found when Cljs calls module_deps.js which uses same dependency resolve library as Webpack

richiardiandrea16:04:20

@juhoteperi there are two reports open against that, one reported by David...might be something else too, weird

juhoteperi16:04:59

Yes. I can reproduce problem with examples in the issues but it also works for me using Lein.

richiardiandrea16:04:26

oh that's good to know, can it be a cljs.main issue then?