Fork me on GitHub
#cljs-dev
<
2020-05-18
>
dnolen12:05:43

@bhauman re: :bundle-cmd this just exactly the kind of thing I'm not interested in working on since it's very unclear what's needed and whatever we choose might not be right in all scenarios - I don't have this problem with React Native in medium size application.

dnolen12:05:11

and it just seems too straightforward to fix it in Figwheel or any other tool if the tool designer wants some specific semantic

dnolen12:05:21

maybe you're agree with this sentiment, not completely sure reading over the backlog

dnolen12:05:53

but I would say the first question everyone should ask themselves is "maybe I shouldn't use :bundle-cmd ?"

dnolen12:05:59

and then your problems go away

dominicm13:05:40

@dnolen just to clarify, the rn bundler doesn't care about your :output-to existing for the first run

dominicm13:05:10

fwiw, I've ended up having to hack something to run in a figwheel post-build-hook which will start a process if it's not already running 🤕. It's a bit hacky.

dnolen13:05:21

right but why bother?

dnolen13:05:28

just start the watcher first manually

dnolen13:05:01

@alexmiller website needs a bump for that AskClojure question

dominicm13:05:24

Because people generally expect to be able to rock up at a project and jack-in without having to open a bunch of terminals to get all of the 5 processes they need running to run a project.

dominicm13:05:39

But also, I don't think the watcher can be started first manually.

dnolen13:05:56

modern JS development isn't like that anyway

dominicm13:05:58

Yeah, it can't. You have to wait until after the first cljs build.

dnolen13:05:01

there's already multiple steps

dnolen13:05:20

and you're using a bundler which you can't abstract away

dnolen13:05:01

anyways just not interested right now, :bundle-cmd isn't going to see any further enhancement

dnolen13:05:07

I said this when I added it

dnolen13:05:11

I'd rather see some other tool work on this problem - discover all the issues before investing any more time in it

dominicm13:05:37

OK. Well, I can sorta do my own bundle for now with figwheel.

bhauman14:05:19

@dnolen I think @dominicm is saying that React Native bundler doesn’t require the output-to artifact at the start while webpack does. So it makes start up a little tricky, ie you can’t pop a browser client open as you have to start webpack after your first cljs compile.

bhauman14:05:29

For this reason I’m thinking that :bundle-cmd should run just once for a watched build, as it handled the awkwardness of getting started and doesn’t incur the unneeded cost on incremental compiles. One can just restart the process, or start webpack with a --watch flag, if needed.

dnolen14:05:10

right but I'm trying to say in this scenario why not just not use it

dnolen14:05:20

start the watcher, run your CLJS tool

dnolen14:05:23

the problem goes away

bhauman14:05:41

I stated why in the preceeding paragraph

bhauman14:05:47

you can’t start the watcher first

dnolen14:05:01

the webpack watcher doesn't just keep watching for index.js

dnolen14:05:11

i.e. the watcher exits if it can't find it?

bhauman14:05:30

so you have to start cljs first

bhauman14:05:51

and then start the watcher and then reload your browser

dnolen14:05:29

what happens in webpack if you delete`index.js` while watching?

dnolen14:05:47

anyways it stuff like this that makes doing anything here super annoying

dnolen14:05:54

which is why I'm not that interested

dnolen14:05:48

or rather see it sorted out elsewhere first

dnolen14:05:54

because it's going to be a time hole

bhauman14:05:56

Yes thats what I was thinking as well, and starting with just bundling on the first compile to facilitate launching repls etc.

bhauman14:05:42

as its a simple straightforward behavior

dnolen14:05:37

but what if you use a different bundler that just doesn't have this problem and you want to build something that just refreshes on npm deps changes etc.

dnolen14:05:49

anyways doing stuff mean less flexibility downstream

dnolen14:05:51

I just don't like it sorry

dnolen14:05:12

:bundle-cmd should make no choices

dpsutton14:05:15

> "maybe I shouldn't use `:bundle-cmd` ?" i was thinking that bundle-cmd will be the default and you need to know that you will never need npm deps to not use it?

dnolen14:05:27

then everybody can make their own and do more interesting things for their other tooling choices

lilactown14:05:29

my 2c: running :bundle-cmd once after the first successful compile seems the most unopinionated way of doing this

lilactown14:05:12

solves the problem w/ tools expecting input at process start, and lets you build the rest yourself if you want something fancier

lilactown14:05:17

it’s onerous to build a tool on top that waits for your bundle to be created and then start another process

lilactown14:05:40

on the other hand, if your tool (like metro) doesn’t care whether it’s there or not it’s still fine to wait until after the succesful build

dnolen14:05:17

@dominicm the other reason this doesn't seem like a real problem you can make your own index.js that includes :output-to

dnolen14:05:38

add a layer of indirection

dnolen14:05:06

or does webpack watcher expect everything to be perfect on the first build - which seems ... wat

lilactown14:05:06

it seems like a simple way to make it robust for the majority of cases

lilactown14:05:07

we can’t control all the different behaviors of the tools out there, but we can control the behavior of CLJS to ensure that code is on disk before it starts up at least

👍 4
dnolen14:05:11

that's not the problem under discussion?

dnolen14:05:18

there is not problem with :bundle-cmd

dnolen14:05:32

what we're talking about is specifically the behavior of webpack watching

dnolen14:05:03

if all you need is a separate index.js so just the watcher can start - there is nothing meaningful for CLJS to do here

dnolen14:05:28

this is exactly how Krell works

dnolen14:05:38

we make an index.js file once

dnolen14:05:00

but you can't do that for ClojureScript anyway because the contents can in fact change

dnolen14:05:07

we could do a similar thing just for :bundle , :bundle-entry or something similar

bhauman14:05:09

This is only for w watched build, and these changes are rare and are the kinds of changes that normally require a restart

dnolen14:05:51

a restart of what? the REPL?

bhauman14:05:03

of the repl build process

bhauman14:05:11

adding a package for instance

bhauman14:05:20

adding a preload

dnolen14:05:25

it's not necessarily true for packages

dnolen14:05:47

In Krell you can just refresh, and the REPL handles reconnects

bhauman14:05:50

things that one would expect might need a restart

dnolen14:05:57

there's no reason that can't happen in the browser too

dnolen15:05:07

you have to refresh, but you don't need to restart build or REPL

bhauman15:05:02

Just trying to preserve the start up experience and the super quick reloads the folks have come to expect, that’s why I’m going to bundle once at the beginning. And they can start a watcher in parallel if they need it.

bhauman15:05:51

there will be a flag if they want to bundle every time

dnolen15:05:02

this seems perfectly fine for Figwheel to do it this way

dnolen15:05:12

I've been saying that over and over again

dnolen15:05:18

this is what I did in Krell

dnolen15:05:47

the tool can make the decisions for the specific integration the creators believes will be most useful

dnolen15:05:53

ClojureScript doesn't give a damn about Webpack

lilactown15:05:04

Testing out other web bundlers, parcel also exits if the entry file doesn’t exist. If a transitive file doesn’t exist, then it shows a loud error (but continues to watch).

dnolen15:05:57

you can use Metro to build web stuff, it doesn't have this problem

dnolen15:05:30

anyways I could go on - but there's just not good reason to do anything just yet IMO

dnolen15:05:35

er, well that's not true, you need index.js

bhauman15:05:08

not surprising

dnolen15:05:25

but this thing is not a ClojureScript file

dnolen15:05:28

it never changes

dnolen15:05:26

part of the issue is really there's several different problems under discussion

dnolen15:05:11

anyways not convinced it's practical to untangle all these things since the specter of convenience / end user expectation is interwined

dnolen15:05:12

I don't really like :bundle-cmd being one-shot because nothing else in the compiler config has this semantic

dnolen15:05:27

:install-deps has the same problem

dnolen15:05:11

and the answer there was the same - you probably don't want it

dnolen15:05:50

the other problem is that these issues are dev issues, :bundle-cmd and :install-deps may very well be what you want for prod.

bhauman15:05:26

my biggest issue is that people lose the current start up behavior (repl connection, browser open) if they opt out of the :bundle-cmd

bhauman15:05:34

so maybe we need something generic an after-build-hook

dnolen15:05:55

for Figwheel this is a UX thing - I get that

bhauman15:05:08

well cljs has the same UX

bhauman15:05:22

it lauches a repl

bhauman15:05:44

oh that doesn’t work with the :bundle target

dnolen15:05:14

it does work

dnolen15:05:24

clj -m cljs.main -c -r variants work

dnolen15:05:45

and :bundle is only going to run once

dnolen15:05:49

so you don't have these problems

bhauman15:05:40

if you add a watcher?

bhauman15:05:56

bundle only runs once?

bhauman15:05:14

anyway its interesting that the REPL works

dnolen15:05:15

no, not going to change this (the watcher case) because of your better idea for hot-reloading

dnolen15:05:36

@bhauman there's a been a bunch of fixes to cljs.cli because of user feedback here

bhauman15:05:48

cool I’m going to check those changes out and get up to date 🙂

dnolen15:05:34

@bhauman another fix is that :main is analyzed now on separate thread when you start REPL

bhauman15:05:54

that was a recent change

dnolen15:05:07

but all of this informed by -c -r

dnolen15:05:26

you compile once, and then you start a REPL, and the REPL takes everything from build

dnolen15:05:38

hot-reloading would answer any remaining issues

dominicm15:05:07

Have you thought about bundle for the hot reload? Would that be run once or run per reload?

dnolen15:05:39

I alluded to this above but probably not clear

dominicm15:05:15

Hmm, maybe I figured it out. But explicit is better :)

dnolen15:05:41

we're not going to run :bundle-cmd except where we run it now

dnolen15:05:49

at the end of a build

dnolen15:05:39

hot-reload generally doesn't need to do anything but the file that changed and its dependents

dnolen15:05:19

the requires may have changed, and we could recompute npm_deps.js which is the required node_modules

dnolen16:05:07

in this case a watch process is going to be better anyway (because it will know that npm_deps.js has changed)

dnolen16:05:51

you will also need a refresh and a REPL that will reconnect on refresh

dnolen16:05:56

so long answer - I'm not sure we should re-run it

dnolen16:05:35

but making :bundle-cmd more useful for watching workflow seems interesting - i.e. we don't wait for :bundle-cmd to return but just start a process (but has a lot of downsides for logging)

dominicm16:05:24

Hmm. So maybe figwheel should clear :bundle-cmd for subsequent builds then - to mirror that proposed behavior.

dominicm16:05:30

Not sure what Krell does.

dnolen16:05:07

it doesn't need :bundle-cmd because you must run npx react-native start anyway

dnolen16:05:28

having that run inside of :bundle-cmd even if we supported it has too many downsides, which why I don't even really like it for the REPL scenario above

dnolen16:05:46

yeah the problem with triggering a bundle watcher inside ClojureScript is directing the output

dnolen16:05:59

the output is also colorized so that has to be configured

dnolen16:05:42

this is true for :bundle-cmd right now too, for very simple builds it's sufficient

dnolen16:05:06

but I'm skeptical about more complicated builds w/ lots of plugins whether you really want to see the failures in a Clojure stack trace

dnolen16:05:52

though maybe we should/could do what RN does - automatically open a new terminal for running that command?

dominicm16:05:52

Personally having one place for it to go (i.e. in my REPL / terminal) is better than checking N places on failure. I actually have non-devs (CEO & designers) working on my Cljs project - so I want to minimize their pain. In a past life I was a consultant, so I made a conscious effort to make sure I could switch projects without having to think about it.

dominicm16:05:26

I... wonder how it does that. At least on Linux, good luck figuring out: - What my terminal is - How to run it to spawn a new command - Whether I prefer tabs, splits or new windows for my terminal

dnolen16:05:20

sure I get that just, throwing out thoughts

dominicm16:05:27

For sure, good to explore everything. I think the only way to work that is something like a ~/.cljs-terminal.edn where I could configure what I want to have happen

dominicm16:05:04

One worry I have here is fragmentation. I know eventually cljs upstream will merge what works. But in the interim we'll have a few competing solutions to this problem. That just makes things more complex for people trying to use react-select in their project.

dnolen16:05:17

honestly that seems unavoidable - and has been true for a long time anyway

dnolen16:05:12

and IMO is in fact desirable - letting tools innovate and folding back in what works is slower but also more sustainable