Fork me on GitHub
#cljs-dev
<
2018-03-06
>
dnolen00:03:24

hrm socket REPLs have me thinking that we should probably be sharing JS environments … need to think about this more

richiardiandrea00:03:04

from my inexperienced point of view it seems that the default-compile stuff is exactly what needs to be executed on another thread for the socket REPL. It seems things in cljs.main can be reused....but again, I don't have the complete pictures in front of me...

mfikes02:03:54

Maybe with 1.10.x, ClojureScript REPLs should align with Clojure, and print the version instead of the bit about :cljs/quit:

$ clj -m cljs.main
ClojureScript 1.10.126
cljs.user=> ▊

dnolen03:03:49

Yeah that would be easy to do, patch welcome

rauh11:03:28

I think we should tag array-index-of-keyword? (and so on) with @noinline. This way JS engines can optimize the functions better with IC lookups (it'll see the same type)

dnolen11:03:34

benchmarks would be good 🙂

dnolen11:03:58

ok this is pretty cool 🙂 Socket REPLs give us a pattern for sharing JavaScript environments

dnolen11:03:01

in a fairly standard way, all Socket REPLs can support this

dnolen11:03:22

i.e you can have many REPLs attached to the same Nashorn/CLJS Compilation environment

dnolen11:03:28

right now testing with browser

dnolen11:03:56

which is all to say we can make the experience more like Clojure

dnolen12:03:10

many REPLs attached to one evaluation environment

mfikes12:03:30

I'm curious if you are adding "REPL isolation" for Socket REPLs, which means that each Socket REPL gets its own *1, and other dynamic vars

mfikes12:03:58

(Like *print-length*, etc)

dnolen12:03:57

no, out of scope for getting the basics working

dnolen12:03:08

somebody can do that as a later patch

dnolen12:03:15

just trying to get the basic pattern established

dnolen12:03:36

but it won’t be hard

dnolen12:03:49

I can have two REPLs and one is one ns and the other is in a different one

dnolen12:03:58

so stuff like that is just details

mfikes12:03:34

Yep, Plank's socket REPL was exactly the same. Got it working, and only added isolation later.

dnolen12:03:10

clj -R:cljs -J-Dclojure.server.repl="{:port 5555 :accept cljs.server.browser/prepl}"

richiardiandrea15:03:27

So hopefully this will solve that Jira ticket, but is this possible only for browsers? Should we apply the same patch to all the other repls or there are other pitfalls there?

dnolen12:03:30

then in different terminals rlwrap nc localhost 5555

dnolen12:03:47

it doesn’t matter which one you quit, as long as one is open the environment is persisted

dnolen12:03:02

once you kill the last one, then rlwrap nc localhost 5555 will launch a new browser

dnolen12:03:13

you can def in different REPL and see those definitions

dnolen12:03:23

no warnings

dnolen12:03:44

(ns foo.core) won’t affect ns of different REPL, etc.

dnolen13:03:00

tickets welcome, I’m not planning on stalling the release for this, but I just wanted to make sure that we have a good shared env socket REPL working that people can kick the tires with - I think hardening socket REPL / pREPL is probably the focus of the next release

bhauman14:03:18

I'd really love to see talk of just making the browser/server repl env more robust period

bhauman14:03:58

it still just simply craps out for no discernible reason

bhauman14:03:52

I read the Quick Start and its funny that I don't see the following sentiment spelled out explicitly, if the main CLJS tools are just toys not intended for real work, this should be stated clearly eh?

bhauman14:03:27

I, also, don't see that sentiment in the tweets announcing the features that cljs.main provides

bhauman14:03:43

Why are we still in this situation making these tools the core of the initial presentation of CLJS while at the same time sharing the unspoken belief that they don't have to be robust because the are just for beginners, or experienced folks who need to test something really quick?

jannis14:03:00

@dnolen I think I’m having a little a-ha moment after what you told me yesterday. Is it correct that basically, we let Closure handle the JS dependencies/modules and then “all” we do is compile the CLJS sources such that they are compatible / in sync with what we think Closure does with the JS files (e.g. module names in goog.provide/goog.require)?

jannis14:03:03

I guess there’s one earlier step in the pipeline where we index / collect the JS files that we think Closure should process.

jannis14:03:11

But is that basically it?

dnolen14:03:52

@bhauman we have be realistic about time and effort and coordination no? That’s what I was implying

jannis14:03:55

@dnolen Pfew, I finally understand something! I’m a little sad it took me so long though. 😉

dnolen14:03:32

@bhauman also I don’t really follow, cljs.main is intentionally an extensible pattern not tied to any particular ecosystem tool so I don’t see what is misleading here?

bhauman15:03:25

@dnolen I'm talking about the Browser REPL and how it continues to be central to our introductory material, which clearly emphasizes its use. And I think it is miss-leading if everyone in the community knows that it can't be used to do "real work", but we don't tell newcomers that. Also I do think that as we raise the browser repl to be the default cljs.main experience, that yes people (including experienced cljs devs) are going to expect it to work better than it has in the past. The disclaimer that "but this doesn't really work that well, this is just for toy examples" isn't there. And it shouldn't be ... it should work very well and developers should be able to use it and rely on it. Yet all these improvements continue apace, let's simply bring the browser REPL (and the watcher) on board for the ride. They are much more important IMHO

dnolen15:03:25

I still don’t really follow, none of the REPLs by themselves are great for real work. They all require non-trivial setup to integrate with web server

dnolen15:03:41

So your narrative here doesn’t seem accurate to me

dnolen15:03:11

Getting into how to use REPLs for real work is not beginner material ... yet

bhauman16:03:37

So you don't think that given the tweets, and the the demos, or that someone who reads in the QuickStart without any knowledge of the ClojureScript community, will not get the impression that we are recommending this as "a" or even "the" way to work with CLJS? And that they could perhaps mistakenly perceive this as the CLJS experience?

dnolen16:03:56

@bhauman I’m just I don’t understand what stance you are trying to communicate

bhauman16:03:56

Fixing the browser repl is a no brainer anyway

dnolen16:03:13

there’s is no REPL I can just use for work right now w/o remembering many steps

dnolen16:03:25

No language Quick Start is going to cover a production quality system

dnolen16:03:47

these sorts of things are always intentionally light weight introductions so you can play around

dnolen16:03:54

I just don’t see how this is any different

dnolen16:03:47

based on the feedback on new Quick Start (from beginners and from people who use Figwheel, boot or whatever), I think this is well understood

dnolen16:03:33

@bhauman what I’m trying to say is you’re stating something which doesn’t seem directly related to a completely different thing - which is “yes the browser REPL needs a lot of work”

dnolen16:03:43

which also completely different from “it needs more features”

dnolen16:03:17

all these points can and should be considered separately

dnolen16:03:13

@bhauman all this said I very much am thinking about making cljs.repl.browser less of a toy - but I’m approaching this from a very different vantage point - making it compose with Socket REPL & pREPL - and ensuring I can add it to Pedestal project or whatever

dnolen16:03:01

I am not currently interested in looking at extending the features beyond what the Clojure REPL provides at this point - i.e. hot-reloading - can think about that at some other point when all the fundamental ducks are in row

mfikes16:03:38

It would be nice to get to the bottom of the inexplicable "does not connect" issue that seems to occur. (Worried about new users being turned away, concluding that "this stuff doesn't work at all".)

mfikes16:03:51

I'll try to dig if it breaks for me.

dnolen16:03:07

@mfikes I haven’t seen that at all, or you talking about Windows?

mfikes16:03:13

No, on my macOS boxes, I get a failure rate that might be around, say 20% of the time.

dnolen16:03:13

@richiardiandrea sharing JS environment is possible for all Socket REPLs, in fact browser REPL is really the only super annoying case

richiardiandrea16:03:20

Ok so let me rephrase the question: should the fix be applied to node repls as well? As you know i am very interested in solving that Jira ticket 😃

jannis16:03:15

@dnolen The funny thing is… both graphql and iterall have .js modules side by side with .mjs, but their package.json files use "module": "index.mjs". I wonder if it would be better and less evasive to fallback to index.js when encountering a package.json like that (and if index.js exists of course).

dnolen16:03:55

@jannis the best course of action is to emulate what Node.js / NPM does

dnolen16:03:10

if it looks at .mjs, Closure should look at .mjs

dnolen16:03:48

@jannis and if you make a PR, clarify that’s what’s being done

jannis16:03:50

Right. I’ll try to find out how I can trace what npm/node are doing.

dnolen16:03:41

@richiardiandrea look at what I did and go fix it 🙂

richiardiandrea17:03:06

so I was thinking about a change that applies that mechanism to all repls, how does it sound? asking because most of the times I am wrong 😄

richiardiandrea17:03:11

the only thing I am unsure of is the lock, with those many atoms and bindings I thought you would not need to lock

dnolen17:03:29

yeah you need to look carefully at what I did for browser (which actually still has some issues)

dnolen17:03:43

threads will race on -setup

dnolen17:03:52

so you need to handle this

dnolen17:03:11

atoms are not enough to manage other side effects

dnolen17:03:24

because the body of an swap! fn can be restarted

dnolen17:03:09

ditto for -teardown

richiardiandrea17:03:23

yep I know, in replumb I employed some trick, let me try if I can send it over to you...or actually...I might just write some bunch of code and see if you like it 😄

dnolen17:03:05

just lock the critical region, and make things idempotent

richiardiandrea17:03:18

the retry always happens and you need to make sure you set a state like :initializing and the next transition function does not do anything when it sees that

dnolen17:03:32

or just use a lock and don’t bother with all that

richiardiandrea17:03:35

yep that is how to achieve idempotency basically with atoms only 😄

jannis16:03:06

node --trace tells me it’s using index.js. Looks like I can drop the .mjs wrestling and just change how Closure & CLJS go from package.json to index.(m)js.

dnolen16:03:31

@jannis isn’t "module" a special thing anyway?

dnolen16:03:42

I thought "main" is the normal way

thheller16:03:11

isn't .mjs still super experimental and hidden behind a flag?

dnolen16:03:18

@mfikes hrm I’ve never seen it fail to connect, not since the latest changes

dnolen16:03:50

even on my pokey 3 year old 1.4ghz Macbook

jannis16:03:50

@thheller You’re right: node --experimental-modules

jannis16:03:34

@dnolen And you’re probably right to. I’m going back to CLJS master and see how far I can get importing graphql and iterall without .mjs files being involved at all.

jannis17:03:19

@dnolen Ah. We explicitly pick "module" over "main" in module_deps.js. That’s why index.mjs is picked up instead of index.js (`"main": "index"` is also set).

dnolen17:03:06

maybe @anmonteiro knows why?

dnolen17:03:20

probably that is reversed?

anmonteiro17:03:11

I think it’s right

jannis17:03:18

If it is, I think we may have to special-case "module" entries that use .mjs files (until those are stable).

jannis17:03:42

If I swap module and main, it picks up all the JS files and most things work (all the graphql modules loaded from .js files are present at runtime, e.g. module$Users$jannis$Work$oss$clojure$clojurescript$node_modules$graphql$type$definition)

jannis17:03:58

Only a few dependencies are broken

anmonteiro17:03:34

we default to ["browser", "module", "main"] to mimic webpack resolution

anmonteiro17:03:18

that sets those options

anmonteiro17:03:23

again, I think ["browser", "module", "main"] is correct, otherwise we run the risk of processing bundled files which may not yet be read by Closure due to the UMD problem

jannis18:03:15

Yep, I agree it’s correct. But if "module" points to a .mjs file then we’d either have to support .mjs files or attempt a fallback to the corresponding .js.

jannis18:03:38

With .mjs being experimental right now, I’d lean towards the latter. What do you think?

dnolen18:03:11

sounds right

dnolen18:03:14

falling back to .js

anmonteiro18:03:27

I think we could support .mjs actually. AFAIK it’s in Node.js Current

dnolen18:03:50

oh so experimental but in Current? Or no longer experimental?

anmonteiro18:03:02

I don’t think it’s experimental anymore. Checking

jannis18:03:07

Thanks 🙂

dnolen18:03:12

flag for the resolution order sounds very tricky

dnolen18:03:17

since you need this at module level

anmonteiro18:03:29

experimental

jannis18:03:35

Yeah, a flag doesn’t sound right.

dnolen18:03:44

so I say fallback for now

jannis18:03:25

Ok, I’ll give that a shot then.

dnolen18:03:27

@mfikes https://dev.clojure.org/jira/browse/CLJS-2620, we just need to make sure quit-prompt usage still works

dnolen18:03:46

and we need a better title for the new flag something more generic repl-title or something

dnolen18:03:51

and we remove default quit-prompt

mfikes18:03:30

@dnolen Yeah, and I was pondering even conditionally wrapping so that nil could be though of as “off”

(when quit-prompt
  (quit-prompt))

mfikes19:03:08

Added another patch, illustrating various use cases of suppressing and/or replacing the title and quit-prompt

mfikes21:03:51

Nice. The MapEntry qualification commit (https://github.com/clojure/clojurescript/commit/ed385ad484f9c6b2471b0ee371fac805d8194c3e) fixed the issue with prismatic.schema (claris.rules) 🙂

mfikes21:03:34

Ahh, cool, I see there was a whole side-thread in #clojurescript about this. 👍

jannis21:03:59

Mmmmh, how do we feel about being able to do the following?

:npm-deps {"/path/to/local/npm-package-0.1.0.tgz" "0.1.0"}
I had the idea after looking at http://podefr.tumblr.com/post/30488475488/locally-test-your-npm-modules-without-publishing and it’s a ~3 line change in cljs.closure/maybe-install-node-deps!. Essentially:
(ProcessBuilder.                                          
  (into (cond->> ["npm" "install" "@cljs-oss/module-deps"]
          util/windows? (into ["cmd" "/c"]))              
    (map (fn [[dep version]]                              
           (if (and (string? dep) (.exists (io/file dep))) ; this is new
             dep ; and this
             (str (name dep) "@" version))))              
    npm-deps))                                            

jannis21:03:47

It allows for an easy way to try a local version of an npm package.

dnolen21:03:11

seems useful to me, enhancement ticket + patch welcmoe

dnolen21:03:24

especially for people who want to test the feature and try things quickly

jannis21:03:00

Yeah, all it requires is a directory with a package.json and at minimum one .js file, so existing (or private) code doesn’t necessarily have to be published via npm first.

mfikes21:03:53

Browser REPL for uberjar built from master is connecting completely reliably for me

while true; do cat repl-input.txt | java -jar cljs.jar; done
just loops without ever failing to connect.

mfikes21:03:44

Oddly (I forgot this fact about Windows): You can just "run" the cljs.jar; Windows knows what it is and has Java run its embedded cljs.main.

potetm22:03:22

I’m super happy the browser repl is getting a little makeover. The new experience is super slick.

mfikes22:03:54

BTW, here is the potential revised copy for the browser REPL:

Welcome to the ClojureScript browser REPL. 

This page hosts your REPL and application evaluation environment. Validate the connection by typing `(js/alert "Hello CLJS!")` in the REPL.

To provide your own custom page, place an `index.html` file in REPL launch directory, starting with this template:
(Trying to make it succinct, without being too terse / curt.)

potetm22:03:40

I’m wondering if we now feel comfortable removing that little debugging note from the docs

dnolen22:03:21

@potetm it’s not in there anymore, @mfikes has experienced spottiness but I cannot reproduce here

dnolen22:03:33

it’s sometimes a bit slow but that’s it

dnolen22:03:15

@mfikes I like it file in *the* REPL launch directory

dnolen22:03:21

@potetm yeah I’m getting a little bit frustrated at it at the moment trying to wire it up programmatically to Socket REPL, there’s definitely a strange race condition in there

dnolen22:03:51

digging into that - over all it could probably use a refactor

dnolen22:03:00

I’m not in love with how it uses bindings to propagate state

dnolen22:03:54

I think multimethods forced the inversion

dnolen22:03:06

could have probably just passed a simple routing table

dnolen22:03:12

and avoided all that

potetm22:03:55

Yeah no, I continually have to bounce around in there. Definitely has some cruft.

potetm23:03:06

And I agree about the routing table.

potetm23:03:53

curious what you’re looking at re: the Socket REPL; I might be able to help

potetm23:03:14

if you have a branch, I can take a peek

dnolen23:03:55

@potetm I don’t have a branch, all the Socket REPL stuff is on master

dnolen23:03:35

if you write a program that tries to connect twice via Socket REPL, you’ll probably be able to recreate the issue

dnolen23:03:49

almost impossible to create manually by starting them at the terminal

dnolen23:03:15

my hunch is this

dnolen23:03:34

first REPL thread calls setup - but this takes time

dnolen23:03:53

second REPL thread calls setup, but someone already started so it skips, but then it gets to the other initialization stuff sooner

dnolen23:03:22

somehow in this scenario the second REPL get locked up

potetm23:03:10

hence the new lock around setup? did that not fix it?

dnolen23:03:31

that just prevents other bugs 🙂

dnolen23:03:27

I think the issue must be something about the second REPL getting to browser-eval first

dnolen23:03:53

If you use tap> it makes it much easier to debug the threads

dnolen23:03:23

but I see the 2nd REPL never gets past the first form for evaluation - declaring (ns user ...)

dnolen23:03:34

this is of course non-deterministic, every now and then it works

potetm23:03:41

Hmmm…. nothing is jumping out offhand. Esp. w/ the lock in place. Like you say, it’s tough to reason about w/ the dynamic vars in play.

potetm23:03:19

My plane is landing right now. I’ll see if I can get it breaking on my box later and go from there.

richiardiandrea23:03:52

what is the lifecycle of the repl? because what if you try to lock the whole thing? just to see if the problem is the fact that -evaluate or -load should also be within the critical section...did not have time today but these problems are fashinating (and frustrating) at the same time

potetm23:03:44

Not sure I follow, but repl-env lives throughout the repl session. As the name implies, setup is called once at the start, subsequent operations are made via on user demand evaluate and load.

potetm23:03:21

In theory, each repl has its own server, so they should not interfere.

richiardiandrea23:03:57

ok I am new to this so thanks for explaining me that 😉

potetm23:03:58

Each server should attached to the browser-env record.

dnolen23:03:48

@potetm right that’s not how it should work though

dnolen23:03:56

one server feeds many REPLs

dnolen23:03:02

you can only bind host/port once

dnolen23:03:09

all REPLs talk to that server

dnolen23:03:23

what you want is shared JS evaluation envrionment

dnolen23:03:35

that’s the only way to get the same semantics as Clojure

dnolen23:03:41

as well as setup way more sophisticated tooling

dnolen23:03:51

you can have all different kinds of REPLs talking to the browser

dnolen23:03:49

there’s no real problems here because JS envs are all single threaded anyway

dnolen23:03:09

but from the user side of things, that’s very powerful

mfikes23:03:53

Draft update for the browser REPL copy in https://dev.clojure.org/jira/browse/CLJS-2623

potetm23:03:59

Right. Love it. Ah so you're reusing a repl env?

dnolen23:03:23

yes it already works and it’s in master

dnolen23:03:26

it rocks 🙂

dnolen23:03:33

except for the concurrency bug

dnolen23:03:47

my feeling is that the connection system in browser REPL is not right

dnolen23:03:02

it’s kind of a botched queue, but I need to examine it more

potetm23:03:14

Yeah I'm not at all shocked that there's a bug there for brepl. I almost automatically lean towards a refactor. I'll take a closer look in a bit.

dnolen23:03:27

@potetm I don’t actually think it needs a huge refactor cljs.repl.server is understandable

dnolen23:03:49

if it just didn’t depend on dyn vars that would be a big step

dnolen23:03:06

on the request side of things it’s really simple

dnolen23:03:18

client connects, accept on the socket and you go through the loop

dnolen23:03:47

I think eval side is where it goes wrong

potetm23:03:20

Yeah I agree.

dnolen23:03:06

I think that the problem is we don’t “allocate anything for eval to use” until we get :result POST request

dnolen23:03:18

fix that and we’re probably good to go