ha @mfikes fast initial prompt is actually pretty nice enabled on browser REPL

it was never that slow - but this makes it feel much faster

Oh, works now. I tested different command and left that in the configuration, with proper npx command I see the output file. I think it would be good idea to print out the webpack output?

Though for watch mode it definitely makes sense to not show the webpack stdout. Anyway, I can just run the command myself if I want to see the output, and probably makes more sense here anyway, as I might want to pass the bundle to webpack directly or to Karma.

RE: not showing the webpack output, I think I've made a mistake in my webpack config and it's taken me a good 15m or so to realize that's why cljs is silently not producing output.

If the command fails, the exception should contain the stdout contents.

Didn't fail, I think I didn't have webpack installed (I guess I wiped my node_modules). And it was prompting for input.

I guess when needing to prompt for input in non-interactive env it exits with 0?

Yeah. And I think it won't show stderr contents.

The implementation closes stdin, so it should exit?

ah, then in that case I guess npx read it as "N", and exited.

Something (possibly) useful is that you don't need a http://webpack.co

Interestingly, while the bundle output is smaller for react/react-dom, shadow still produces slightly smaller output. (1KiB difference).

Should ^:export work in advanced optimized bundle, so that the exported fn can be accessed from the JS side? Looks like goog.global is not the same as window and looks like it is not seen outside of Cljs module.

Maybe Cljs should set goog.global = window somewhere, goog.string.unescapeEntities also breaks as it doesn't find goog.global.document.

https://github.com/google/closure-library/blob/master/closure/goog/string/string.js#L569

With few inferred extern additions, I now have Reagent tests passing with advanced optimized bundle.

@juhoteperi https://clojure.atlassian.net/browse/CLJS-3227

@juhoteperi @dominicm re: :bundle-cmd output I really don't care that much here - it's just important to note it's a convenience

as noted you can easily run the bundle cmd separately and that might be common because you want to do more things

and the :bundle-cmd is intentionally simple and doesn't support parameterization

not interested in that at all

One problem is that I don't understand how bundle relates to development mode at all

I don't know what you mean?

Well, I have this "worry" that it'll be run more frequently than just once. So dev would be very inconvenient otherwise.

Bundle-cmd is run after every compile. Or I guess you could use JS tools to watch for file changes.

How often is a compile run?

e.g. on file change?

In watch mode, on file change.

Okay. One worry I have is webpack failing mid-development session. That would be a bit of face meeting wall to realize/debug.

that possible but I don't think it's much of a worry - you need the bundler when your ClojureScript requires include something from node_modules you haven't required before.

also nothing is stopping anyone from using watchman and setting this up however they like

:bundle-cmd is not important

@juhoteperi we could change ^:export here, goog.exportSymbol takes a third parameter - I'm wondering if that's good enough

Closure library code which presumes goog.global is the window object is also a problem, that wouldn't help with that

@juhoteperi ah yeah, ok we have the hook for this in core, only used in Node.js right now

@juhoteperi try master

if that works for you will close 3227

can we please not do this?

window does not exist in web workers

react-native also doesn't have window

there are plenty of things that might want to use bundle but don't have window

Maybe webpack has some way to handle this?

the "problem" is that goog.global = this breaks because this is no longer window after webpack has processed it

assigning goog.global in cljs.core is also too "late" given that it may have been used before loading cljs.core by closure code (eg. goog/base.js)

in shadow-cljs I overwrite it directly in goog/base.js before emitting that file

https://github.com/thheller/shadow-cljs/blob/94c99bd609bf674e24b47964d37c89e57e838414/src/main/shadow/build/output.clj#L491

Cljs should probably use similar logic to select the global object?

thats not actually the correct code

that is used for :npm-module which is a rather special :target in that it exposes each CLJS ns as a separate require("shadow-cljs/cljs.core") ns

but if you assume webpack then I guess that would work regardless

but I think the next version of webpack will remove the process polyfill so that would break then

@dnolen Export & unescapeEntities works now (but I had other problems with Karma)

maybe instead of checking for process.browser the global object could be just window || self || global ?

@juhoteperi I don't really know what you're trying to suggest

^ related to @thheller comments

I think automatically checking for those cases is a bad idea w/ limiting to target

Yeah, maybe it is best to leave goog.global intact by default? MAYBE option to set it to something IF webpack or other tools don't have better way handle this. It depends on where the bundle is used, what the global object should be.

Another option instead of mucking around with goog/base.js - we could just make a new compiler option to set this

independent of target, because I see that conflating is an issue

@thheller I've never personally seen cases where setting target at the end of core.cljs is a problem - do you have something specific? this after implementing in a bunch of environments, browser, Node, React Native etc.

might be less of on issue today with the new goog.define logic

but it used to be a problem that the goog.define in goog/base.js would then be defined in another scope

yeah it's not .a problem as far as I could tell

Webpack has way to set module this to window: https://webpack.js.org/loaders/imports-loader/

@juhoteperi right but this will be different in every bundler - I'm ok w/ having a way to control this because ^:export is critical

Yeah. Separate compiler option sounds ok to me.

https://github.com/clojure/clojurescript/commit/f95a13b3e3b193749e10e15d95ef98506ce3be29

no new compiler option, it's just a new goog-define

only supports string value: "window", "global", or "self"

@juhoteperi let me know if that works for you

@dnolen It works

great

draft announcement for the new bundle target - https://github.com/clojure/clojurescript-site/blob/april-release-target-bundle/content/news/2020-04-13-bundle-target.adoc

feedback welcome!

a lot of the changes/fixes to master were based on a new React Native tool I've been working on for the past three weeks

https://github.com/vouchio/krell

if you're curious how to leverage cljs.main and the new build target in a custom build tool - this is worth checking out 🙂

re: krell, very cool. it looks like it doesn’t support hot-loading yet; do you use a REPL workflow atm?

I’m v interested in creating an easy hot reloading experience that takes advantage of the new react-refresh package. It’s a little tricky tho.

react-refresh requires that when changing a file, all files in the dependency tree up to the root will be reloaded. This is to support e.g. changing a function or constant in a file, that gets required by another file, that then gets required and used in a component, will properly invalidate the component and trigger React to re-render that component. figwheel and shadow-cljs typically takes the simpler approach of only reloading the file changed and it’s direct ancestors because we assume everything is in the global scope and that we don’t do side-effects on reload (which react-refresh is essentially a side-effect on reload)

hot-loading is under development, but this ClojureScript so it's trivial

re: REPL - that's mostly what Krell does - REPL stuff - the bundler stuff is just reusing ClojureScript master

I feel like if I could figure out how to handle these react-refresh cases from a REPL, it would make everything else easier

the “side-effect on defn ” pattern that react-refresh heavily encourages makes it more difficult

@lilactown I think the only thing is to not swim upstream

Krell auto-reconnects after a React refresh

so what you're talking about probably just works

hmm not sure yet

the point is you cannot preserve the REPL state - but depending on what you're doing who cares?

the point of auto-reconnect is that you can just follow all the usual React guidelines

no divergence

yeah, I want to preserve the behavior you get when enabling “Fast Refresh” in React Native

I'm not saying of course that react-refresh works - just saying that's definitely one of things I'm interested in doing different w/ Krell

that is if you want to use react-refresh Krell should get out of your damn way

disabling React refresh should be entirely an optional thing because you want to do a stateful REPL session

yep, mostly thinking out loud hoping to glean some good ideas 🙂

I think there’s two kinds of refresh in RN: a full reload of the app bundle on change, and the new “Fast Refresh” feature IIRC

my good idea - don't mess up React stuff and React workflow if users want it

no other good ideas

The “Fast Refresh” mode preserves state across reloads - even local component state - but requires you to generate signatures for each component that get invalidated on file change. this is done by a babel plugin that RN gets shipped with, which obviously won’t work with CLJS

a react wrapper lib (like helix/reagent/etc.) can generate these signatures using a macro, but requires integration at the reload step to properly invalidate them like I was saying

I dunno, just thinking out loud. thanks for listening

oh because they process the JS

yeah, I dunno I'd have to look more closely at how it works to have any clearer thoughts

@lilactown but one question I have is how is react-refresh really any better than just developing a RN Reagent app over a state atom w/ Figwheel / shadow / soonish Krell?

yes, IMO

the ability to reload while preserving local component state is huge

why?

if you write all state into an atom same result no?

it completely changes the way that the DX encourages you to architect your app

In a web app this often not that fun - but for shallow view graphs in mobile I don't see the problem

if you take the stance that local state and global state are functionally and architecturally identical, I agree with you.

that's all I'm interested in though - if the end result is same performance, same experience

what do you need the distinction for?

IME the performance and experience is not the same

I haven't followed along but you also have Om inspired projects - the reconciler does what you're talking about

Om's idea is that you have global state - but the reconciler gives you precise updates

especially with concurrent mode unlocking more perf and experience knobs for devs, which works best with local component state

like I said I don't have a strong opinion and I personally think there are alternatives

reading over https://reactnative.dev/docs/fast-refresh it doesn't seem very simple and lots of caveats - so meh

Haven’t dove down to fast-refresh for React Dev yet, but I can comment on the state bit - my goal is to take advantage of the new React Concurrent mode when it lands. So far, and until some new docs land, React wants to you let it handle state (there was months of churn to figure out how this could work with global state).

This also impacts you more the more you rely on hooks, and the more you rely on JS components that use hooks. Fast-refresh promises to keep state from hooks and rerender, for free. Seeing that esp. for UI work local state makes some sense, preserving that makes sense.

I don’t have much more insight :)

@orestis yeah I haven't followed the Concurrent mode stuff that closely - it does look interesting - but I don't know what you mean by "React wants you to let it handle state"

you write a functional component, you use a hook? what aren't you handling?

what he means is that, Concurrent mode works best when your state is coupled to an instance of a component.

construction/destruction relies on mount/unmount, updates to state mean the component re-renders, etc.

concurrent mode allows React to pause rendering a tree and resume rendering that same tree later. this creates problems if the state that was rendered in multiple parts of the tree has changed in between pausing and resuming

React can handle this in a transactional fashion if you manage state inside the component instance. it essentially puts state updates on a queue that gets computed on the render that it triggers. if you store state outside of a component instance, then React can’t handle those state updates for you - if you update an external state atom in a naive synchronous way then you can end up with parts of your tree that were rendered using an old atom state, and parts that were rendered using a newer atom state. if you store state once, and it lives in a component, then resuming the tree will either re-use the state updates already computed inside that render, or compute those state updates against the latest state. you avoid the sort of coherency problems that state external to the tree can introduce.

react is adding more tools to handle external state but the happy path is definitely storing it inside a component using useState / useReducer etc

yeah gonna have to look at the docs more - thanks for summary's though - helpful

The most annoying thing is that this is very much in flux. This approach for examples invalidates all the major React global state managers (redux, relay, Apollo). There’s an RFC called useMutableSource IIRC that seems to handle this, and to my knowledge @mhuebert did a POC some time ago for ClojureScript. I’ve kind of decided that I’ll wait until an official release or more guidance is available.

My only fear is that as with fast-refresh, the React devs will end up with just a Babel/webpack plug-in that we will have a hard time incorporating into our dev workflow.

yeah that was my immediate impression

not compatible w/ existing libs

in fact that's the part that I find kinda busted about it

If I had time to focus on this I’d definitely try to reach out to the React team, they said they wanted to release concurrent mode under an experimental release so that tooling can catch up. Sadly my day job leaves zero time for this kind of low level stuff.

why not just make a new library - don't call it React

Heh, JS doesn’t work like that. React-router is in its 6th or 7th version.

they have lots of paths for migration

But to be fair to them, they are trying the best and using the new Facebook as a guinea pig.

The plan AFAICT is that the official release will be simultaneous with major libraries so the change for consumers will be mostly under the hood.

well this still seems a bit far out to me and upgrading legacy applications seems like a monumental task

so not much bearing on a RN REPL at this point 😛

yep 😛 all of this CM talk is just motivation for why I desire good react-refresh integration

but it sounds like I can hopefully build it myself if need be

it works quite well with shadow-cljs right now too

I expect that libs and apps that use non-CM safe patterns will continue to be supported for a long time. there are a lot of different “modes” that allow devs to tune React to what works with their code

Actually the new bundle stuff sounds like we can finally start releasing tools that depend on React and other npm stuff more easily.

the benefits to adopting CM are mainly around unlocking better UX

@orestis yes that's the point

Super happy about it, thanks

I totally wish React had put Concurrent into a new lib with different name. I am just glad that this useMutableSource stuff is finally arriving. Looks to me like it is going to work well.

@dnolen was there a tools/repls page that got removed on the cljs site?

@alexmiller which one? I deleted a lot of stuff

yeah, guess so - that was the top nav link for tools, so this broke a bunch of nav

https://github.com/clojure/clojurescript-site/commit/1aff4034a39b83fddb3472bb5df140c7603604bd

yeah that's just way out of date don't want people looking at that anymore

that's before we switched to 1.X naming I think

@alexmiller you're not talking about internal links right?

I can clean that up - but nav stuff I don't think I can

I'm talking about the top level nav - the Tools link went to that page

I'm fixing it up now that I understand what happened

I thought I broke it :)

ah k, I didn't understand nav worked that way - it shouldn't have been the top link at this point

I can easily add redirects from old links to somewhere new too if you run into that, there's a list of those in the deployment

should be fixed up now - there's now a tools/tools page which is in the clojurescript-site that mostly matches the left nav

k thanks!

@dnolen not sure if you are monitoring ClojureVerse, some comment left there on Node versions: https://clojureverse.org/t/test-drive-the-beta-clojurescript-js-bundler-integration-support/5779

@orestis chimed in, not sure what's going on there seems similar to what @juhoteperi but I think missed the root cause

I did the tutorial with Node 10.16.0 myself

Cool I didn’t know if you had an account :)

Related to JS modules, I've been already thinking about adding notice to Cljsjs page and repository strongly suggesting using other ways to consume JS libraries. Now is even better time for that.

@juhoteperi yes, but let's wait for the release

then it doesn't matter what tool people use they can follow that guideline easily

I close this one old module processing issue, couldn't reproduce the problem now: https://clojure.atlassian.net/projects/CLJS/issues/CLJS-2836

👍:skin-tone-4:

I think https://clojure.atlassian.net/browse/CLJS-1543 this is now done (goog.module support)?

yes