Fork me on GitHub
#cljs-dev
<
2020-05-15
>
Oliver George00:05:36

Hello. I noticed a problem with the REPL resolving aliases in certain situations. Here are my repro notes, tell me if I should raise a JIRA ticket. https://gist.github.com/olivergeorge/37f420d369e3a4d60c5bf5ff2c171664

dnolen00:05:54

@olivergeorge I would say none of these cases are a problem

dnolen00:05:14

you have to set :analyze-path

dnolen00:05:52

also doing anything here by default may effect existing tooling or expectations

dnolen00:05:37

would need to collect some information

dnolen00:05:55

a safer way would be something new like :analyze-main and only do it for cljs.main

dnolen01:05:01

@olivergeorge hrm I think restricting this to behavior cljs.main could work

dnolen01:05:11

give master a try - if it turns out to be problematic might revert - but it seems like it could work

dnolen01:05:25

if REPL gets :main then we analyze it in a separate thread

dnolen01:05:00

@mfikes related, I can't remember why we didn't analyze-path off the main thread

mfikes01:05:04

@dnolen It was to gain more parallelism, IIRC. You can analyze in one thread effectively, while emit in the other.

dnolen01:05:06

@mfikes sorry you're probably missing the context

dnolen01:05:24

REPL has :analyze-path feature so that you can resolve stuff you know you compiled

dnolen01:05:36

but this happens on the REPL main thread while starting

dnolen01:05:41

so it slows down startup

mfikes01:05:06

Oh, sorry, I read that but I read it as "analyze". Hrm. I don't recall anything around analyze-path off the main thread.

mfikes01:05:03

Ahh, so you are wondering if it is safe to put it off to the side and re-join with its results after the REPL is live. That sounds like a good tactic to reduce latency-to-first prompt, yes.

dnolen01:05:18

@mfikes there's actually no need to rejoin 🙂

dnolen01:05:26

because it runs via bound-fn

mfikes01:05:33

Ahh, I forgot we are working in a sane language.

mfikes01:05:28

(I'm often afraid of the unpredictable effects of these concurrent threads, but immutability saves the day.)

dnolen01:05:13

let's see how this change for :main fares

dnolen01:05:38

if it seems ok then we could do it for :analyze-path

Oliver George01:05:18

Looks like that's done the trick. Thanks.

Oliver George01:05:39

Specifically, my repro now works as expected against 00079768f9

dominicm13:05:59

pretty minor thing: the webpack config doesn't really need to exist. It can be replaced by the cli entirely, meaning no need for a (complex looking) file.

bhauman13:05:31

yeah ["npx" "webpack" "infile.js" "-o" "outfile.js" "--mode=development"]

bhauman13:05:30

it might be better for templates and tutorials to have that

dominicm13:05:04

@bhauman yeah, I was just digging that out :)

dnolen14:05:35

@dominicm PR to the guide would be great!

dpsutton14:05:44

would be nice for both to be listed in the PR rather than just the smaller cli version

dnolen14:05:43

@bhauman I added reconnect to browser REPL, that said I could't get the browser REPL to disconnect spuriously - probably because we had some nice fixes a while ago that made things more stable.

dnolen14:05:58

@dpsutton like documenting both ways?

dnolen14:05:05

in case you need to expand the config easily?

dominicm14:05:24

@dnolen added to my todo list

dpsutton14:05:33

right. its nice to have the quick version when just getting a hello world version. and then when you need to expand that for a real world app nice to have the full config that's equivalent in case it needs to be a bit more complex

bhauman14:05:02

agreed as well

bhauman14:05:23

@dnolen I thought more about the polling repl idea

dominicm14:05:24

What sort of "real world" things need to go in there. I've always just left it alone after creating it (with the old version of the guide).

bhauman14:05:53

the file should be a truncated log

dnolen14:05:20

@dominicm just that if you have a project and you need to add plugins etc.

dnolen14:05:40

it took me a surprisingly long time to figure out that "simple" config

dnolen14:05:45

reading webpack docs is a nightmare

dnolen14:05:50

or was then I put that together

dominicm14:05:05

Still is 😂

dnolen14:05:28

@bhauman not seeing the rationale bit - elaborate? just to avoid logic, you can just eval it?

dominicm14:05:35

Okay, actually. These 2 things might be related to what I'm currently doing actually. I've got some legacy ESM that I'm integrating into my project. Should I be running that through webpack somehow? Or through the alpha JavaScript modules w/ babel.

dnolen14:05:16

@dominicm I would avoid the alpha modules feature for now

dnolen14:05:51

one problem you'll have is if you want to use the modules from CLJS and they're not in node_modules

bhauman14:05:08

@dnolen just to prevent double evals, missed evals

bhauman14:05:44

this could allow multiple evals

bhauman14:05:54

=> 1 2 3 4

dominicm14:05:00

I have ESM with jsx in, how should I include that in my project?

bhauman14:05:29

its just more robust

bhauman14:05:37

for little cost

dnolen14:05:35

@bhauman I'm not following how is this related to hot loading? oh you mean like multiple compiles triggered by N expressions?

dnolen14:05:28

@dominicm so you need what I said, to require your module into CLJS

bhauman14:05:21

I was being specific to a REPL that operates over client polling, hot reloading via a changes file is less critical

dnolen14:05:00

@dominicm the easiest way right would be to get that into your node_modules first - i.e. make it package

dominicm14:05:58

@dnolen and then it has a build step in making it a package which turns it into cjs or something?

dnolen14:05:16

@dominicm no you don't need to change anything about your code

dnolen14:05:35

but it needs to be in node_modules we don't support relative paths

dominicm14:05:08

Then cljs will generate a require() for it, and then webpack will be in charge of that?

dnolen14:05:49

@bhauman I'm not following anymore - sorry more focused now. I don't really see a need to change the REPL more generally?

dnolen14:05:16

there's nothing about the existing browser REPL strategy that poses problems

bhauman14:05:23

ok maybe I missunderstood you yesterday

bhauman14:05:48

cool then I rescind the proposal 🙂

dnolen14:05:03

browser REPL works fine - automatic reconnect addresses reliability

dnolen14:05:21

yesterday I was talking about hot-reloading

dnolen14:05:34

that the changes file is an easy thing to do for anyone

dnolen14:05:40

browser REPL can just use it

dnolen14:05:21

and I like polling for that a lot because - file watching involve a bunch of stuff I don't want to do

bhauman14:05:21

how specifically are you envisioning it using it ?

bhauman14:05:45

but once you find the file?

dnolen14:05:05

cljs.hot-reloader

dnolen14:05:25

just fetches a file, we know goog.basePath so we know where it is

dnolen14:05:38

when it gets a new file it calls goog.require that's it

bhauman14:05:39

oh but you can poll from the client right?

dnolen14:05:46

yes no server stuff here

bhauman14:05:50

why form the repl

bhauman14:05:56

why poll from the REPL?

dnolen14:05:57

that's what I thought you were taking about 🙂

dnolen14:05:04

not from REPL, from client

dnolen14:05:19

{:preloads [cljs.hot-reloader]}

bhauman14:05:25

when you say browser repl

bhauman14:05:31

I missunderstood

dnolen14:05:34

forget about REPL 🙂

dnolen14:05:42

just a hot reloader

bhauman14:05:48

understood

dnolen14:05:52

browser REPL would just add that preload

bhauman14:05:25

absolutely we are on the same page for sure

bhauman14:05:56

OK so another suggestion, include compile excpetions, and warnings in the file

dnolen14:05:50

[{ns-sym desc-map ...}]

dnolen14:05:22

well timestamp somewhere

bhauman14:05:28

yep and one more thing

bhauman14:05:11

would be super nice mark which files were actually changed versus dependents

bhauman14:05:01

thats just a detail but its nice to know which files the user actually changed versus which are dependently compiled

bhauman14:05:37

but this is a nice to have and can be a future improvement

dnolen14:05:21

feedback appreciated

dominicm15:05:27

;; needed for advanced but also fine in dev I assume? (from https://clojurescript.org/guides/webpack)

dominicm15:05:27

Just to double check, I know there's the $ stuff in the works (which would be PERFECT!), but for the meantime is this considered valid? (require '[front-end :as front-end]) (front-end/utils.Auth.get "/some-url") Where utils is a folder, Auth is the default export of Auth.js (an instantiated class) and .get is a method on that class? I'm guessing I might need a .default in there.

souenzzo15:05:49

about npx docs It always take the "newest" version of the thing and in JS community, break things is almost a "good pratice" ["npx" "[email protected]" ...] should avoid problems in the future (but it still can be problematic, once it pins only webpack, any subpackage required by range can break in the future

dnolen15:05:12

@dominicm fine for dev too, yes that require should work

thheller16:05:05

what? that is absolutely not valid?` utils.Auth.get only does property access, it will not look for files, folders or anything of that sort? it is valid if front-end exports the object structure like that but not otherwise

dnolen16:05:11

oh sorry I missed the end of that sentence

dnolen16:05:29

yeah that won't work

dnolen16:05:54

@dominicm you really need to :require the same thing you would in Node.js

dnolen16:05:29

I guess in this case maybe something like "front-end/utils/Auth" I'm not that familiar with conventions around exporting a Node module name

dnolen16:05:03

but I'm assuming that relatively easy to sort out

dominicm17:05:01

@dnolen yeah, you're right. Just figured that out. Then it's Auth/default.get I guess. I feel the need for $ more now. I think the generic solution is the right one.

dominicm17:05:16

Where we're not using default it's quite beautiful though :)

dnolen17:05:04

that feature will definitely appear in the next release

dnolen17:05:49

There's a lot of TS code out there and probably a lot more DCE friendly

lilactown17:05:36

yeah I got super excited about it

lilactown17:05:50

there’s sometimes where I really want to drop down into JS and write some super performant/mutable stuff. TS makes it way easier to write JS. having it hook into closure would be 🚀

👍 4
bhauman17:05:18

@lilactown I’m sure you are aware of this already, but you can do that now, by writing a goog module compatible file, on the correct classpath?

dominicm17:05:48

I think the key part was that TS makes it easier to write :)

lilactown17:05:49

yes, closure JS is very tedious 😛

bhauman17:05:41

hmmm interesting

bhauman17:05:44

goog.provide("my.ns"); my.ns = function() {}

bhauman17:05:57

thats tedious 😉

dominicm17:05:08

Oh man, that syntax is so 10 minutes ago. Now we just write $12##$!

12
lilactown17:05:45

it’s mainly the typing

lilactown17:05:12

I actually enjoy TS’s interfaces type language

👍 4
dnolen17:05:12

heh I'm less interested in the JS writing part - which yes we need to improve, and more into the there's all this TS we can we use

lilactown17:05:22

yeah for sure

dominicm17:05:24

I'm trying to find the xkcd about how humans can find anything interesting, if they're locked away in a box for an extended period for example. But <insert that>

dnolen17:05:28

I only expect TS to get more and more popular

dnolen17:05:56

and because it's Java/C# like it mean libs are less likely to do things that are overly dynamic

dnolen17:05:18

plus Google is almost certainly going to go there anyway

lilactown17:05:22

I think someone in a JIRA ticket pointed out https://github.com/google/incremental-dom

dnolen18:05:22

huh webpack does tree shaking of ES2015 import now https://web.dev/commonjs-larger-bundles/

dominicm18:05:39

Makes sense, that was one of the goals of esm.

dominicm19:05:17

Is this an obvious "oh, you've done X" mistake? Caused by: java.lang.Exception: Could not write JavaScript nil

dominicm19:05:37

Coming from write-js?

dominicm20:05:22

Looks like find-sources is returning an empty list, which add-preloads does a (first post) on and gets nil. I'm not sure if -find-sources should be returning an empty list or not? I have no :modules in my opts (from a prn), and that seems to populate something of importance to cljs?

dnolen21:05:43

not obvious - might need something minimal here

dominicm21:05:47

blugh. I'm definitely in confused territory. That error doesn't happen with figwheel - but now I'm not getting an npm_deps.js produced, even though it's required by the main .js file...

dominicm21:05:57

Something has gone very wrong :)

dominicm21:05:13

@dnolen RE my first error, I can reproduce in mies by: bumping cljs & removing the "src" from build.clj

dominicm21:05:36

Fixing that in my build.clj (by providing "src") gets me to the same place as figwheel, but now I can inject printlns :)

dominicm21:05:25

@dnolen RE my second error, npm_deps.js is written, but was not a sibling of my output-dir. I was using:

:output-to "target/dev/public/frontend-output.js"
         :output-dir "target/dev/public/frontend.out"

dominicm22:05:00

Parcel.js is an order of magnitude (4s vs 300ms) faster than webpack, and seems to produce js which is half the size in dev.

dnolen23:05:42

@dominicm I'm not completely following but if you believe there's a bug - it should be repro'd w/ just ClojureScript