Sweet, one way to judge the success of António’s “ClojureScript Is Not An Island” post is that it made it into the top 10 Hacker News “ClojureScript” posts of all time

Hrm I suppose that 2nd analyze call might be needed https://dev.clojure.org/jira/browse/CLJS-2242

I wonder if we're just not loading macros on the first call or something

Summary of checked arrays for downstream Figwheel use: - Patch landed there fixing its misuses: https://github.com/bhauman/lein-figwheel/commit/48eba2f7e481be4079fdfc4b606d3eb52217a891 - Patch exists for core.async (which Figwheel uses) https://dev.clojure.org/jira/browse/ASYNC-201 This should be sufficient for mainstream use of the feature. The only remaining issue is that Figwheel transports *print-err-fn* args using edn https://github.com/bhauman/lein-figwheel/issues/575

closing in on comprehensive support for all JS module types at the REPL in non-Node.js context

@dnolen let me know if there are any areas you think may be flaky and could use help with manual testing

I think all the obvious issues are solved now - I left a lot comments on the trail

later we probably want to rethink how we do this

I’m testing this under Nashorn/Rhino right now

I'll be really surprised if those just work

Esp. Rhino

I’ll be mad if they don’t just work since we spent a lot of time on this before 🙂

Good luck!

Nashorn just works

I think we might be done wrt. self-hosted too

The global exports patch you applied this morning makes every new feature work downstream in Lumo

Rhino just works

This should mean all downstream REPLs (Figwheel etc.) now get this functionality for free

nothing special for them to do

all JS modules types can be hot-loaded

Yeah

That's awesome

I wonder how many JS people out there really understand what we have going on here wrt tooling 😛

not many 🙂 but we should market that part better

Hopefully the blog is a good start

yeah

ok master is in a good state far as I can tell, tests passing, and I couldn’t break loading JS modules in my simple testing

there’s probably some finessing we could do - but everyone should try it out and bring feedback

How hard do you think it should be do add some REPL tests?

At least in a Node env

Maybe later use Chrome headless for browser repl now that it's a thing

hrm, probably not hard if we just setup some infrastructure first

I don’t think we need to bother with browser REPL actually

Nashorn + Node.js would be good enough

Browser/Nashorn/Rhino code path is all the same

Node.js is separate because of :nodejs target

I bet you feel this pain more than I do but we've broken REPLs on several occasions without knowing

yeah, automating REPL testing would be really sweet, it’s just an really easy step to miss

there’s an existing ticket for it

I don’t think it will be that hard

REPLs are just over standard in/out

Cool, I may look into it in the near future

just needs some scaffolding to make it easy to write test cases for

Testing has also been a theme for this release and I'd love to keep it that way

agreed

Really satisfying to see all these green ticks https://github.com/mfikes/clojurescript/commits/master

One nice side effect of that particular test env is it is an older JSC

heh just realized :npm-deps should work just fine for Ambly too 🙂

Whoa. Mind blown

I’ve been testing with {:npm-deps {:lodash "4.17.4"}} that’s all you need to do w/ your REPL options

There has to be a way to fix that

Not without some fiddling I imagine

It is the wild west

We'd need to make it aware that React is Closure's React

Somehow

Maybe through a CommonJS shim or something

hrm I don’t think so

We actually might be extremely close to RN working with all of this. It is a Rumsfeldian issue right now.

far as I know you just need a custom module loader

Haha

I think if you had a module loader that understood Facebooks weird JSDoc @provides thing you’d be good

Hah didn't know they still used that

but actually … maybe not even?

one thing to realize is that we refuse to get into the relative include madness

but yeah somebody needs to mess around with it

For me to contribute, I have to first understand what re-natal is doing to make things work. I used to understand natal / Ambly.

My mental model is that we may have 2 copies of React if we're processing it and the Packager is loading another copy from node_modules

right I’m suggesting forget Packager

Not sure if you can do RN without the Packager

Hrm but I saw something that might be of interest the other day, 1 sec

you will need a custom view with your own loader thing

but getting rid of Packager never seemed itself be a real problem

custom view with your own loader is more annoying to do

https://github.com/callstack-io/haul

^ I think this replaces the pckager

And might be easier to integrate with

Right, someone in #cljsrn mentioned that thing. I’m starting to feel the tool fatigue 🙂

Yeah this might be a better conversation for that channel

If it solves the problem, I’m all for it 🙂

Also, perhaps Expo will solve the problem if there is money

@anmonteiro that package seems to imply this would be feasible now?

no need for custom view

Where we stand right now in RN land is a bit fragile. It barely works, and whenever Facebook releases, it stands a good chance of breaking.

heh wow Figwheel error reporting has really gotten good

testing Figwheel :npm-deps now

Yes. Right after we added that Levenstein distance suggestor in ClojureScript, Bruce out did it by a mile

Figwheel just works

so cool.

@anmonteiro One thing I’ve noticed, if I try to use :npm-deps, my package.json gets updated so that the line reading

"react": "16.0.0-alpha.6",

"react": "^16.0.0-alpha.6"

I’m going to switch away from yarn to just npm. Maybe that is exacerbating things.

I don’t think it is

that’s probably the result of doing npm install [email protected]

@dnolen do you think we should npm install --no-save?

that wouldn’t save deps in package.json

I have this, FWIW:

:npm-deps       {:react "16.0.0-alpha.6"
                          :react-native "0.44.2"}

I don’t know what the most sensible default would be

The background of this: If you set up a new app using re-natal and it sees that you have yarn installed, it sets things up with a yarn.lock file, etc.. I’m starting from that working state, and adding :npm-deps as above and seeing if I can then make use of it.

the problem of using :npm-deps & yarn is that one will overwrite the other

like, there’s no way you can use the react you installed from yarn if you specify a new one in npm-deps

yeah… I’ll simplify by removing yarn for the time being

@anmonteiro I think current behavior is OK as default + we need a flag?

flag for what?

--no-save

I don’t really care that much what the default is, just that we choose one

and have a flag to set

here’s a different suggestion: a flag that doesn’t install modules, but uses a node_modules installation if it finds one

then people can use yarn if they please

then we provide the blessed :npm-deps path which saves by default

and people that want more control can tweak their behavior

I quite like the idea, let me prototype something

k

@dnolen should be OK to overload :npm-deps to accept true too, or would you prefer a new compiler option?

hrm the problem would be upstream npm-deps

maybe this could really be a big hammer in that you’d need to specify overrides for those upstream deps too

I can see that being useful for people that know what they’re doing

I don't think we need to do that now. Separate option please since :npm-deps is a deps.cljs thing

@dnolen https://dev.clojure.org/jira/browse/CLJS-2245

surprisingly clean now that we can index the top level node_modules directory

then users can use whatever build tool they want

@mfikes mind giving this patch a try?

Sure

simply add

{"devDependencies": 
  {"module-deps": "*",
   "resolve": "*",
   "browser-resolve": "*"}}

to your package.json

along with your other deps

then replace the :npm-deps entry in your compiler options with :node-modules true

OK

it should now respect your yarn.lock file

(make sure you install the dependencies yourself with yarn install)

Yes

👍

@darwin Is Canary actually building master now? (The version numbers look accurate. 🙂 )

@mfikes almost there 🙂 in general it should be able to build any fork you point it to

Wow!!!

@mfikes maybe you could look at env it is passing to project travis builds and prepare planck: https://travis-ci.org/binaryage/cljs-devtools/jobs/253987756/config

this is what I did in cljs-devtools, to make it participate: https://github.com/binaryage/cljs-devtools/commit/45c1df1e0de53c9d320963b296bd7a741056599c

I suppose I can mess with Planck’s .travis.yml — if it is building in that particular environment, I’d need to sort out the fact that it does language: c and has osx and linux-specific setups

wait, I want to trigger builds in your CI, I think there is no need to change anything, just make sure that CANARY_CLOJURESCRIPT_VERSION gets used

(Planck has a build matrix that fans out to 4 separate things, if you include gcc vs clang)

Oh, OK, easy enough

I’ll push in a change that looks for that env var

ok, we can pass in some travis params to build only one of those builds in matrix

this is another thing you should look at, you need to pull that jar https://github.com/cljs-oss/canary/blob/master/scripts/install-canary.sh

That was my next question 🙂

I did this in before_install https://github.com/binaryage/cljs-devtools/commit/45c1df1e0de53c9d320963b296bd7a741056599c#diff-354f30a63fb0907d4ad57269548329e3R18

OK, so if I add that line to Planck’s before install it would take care of getting the JAR?

yes

and it will be future-proof

Ahh, and I see you can do def in project.clj… one sec and I’ll push that stuff in 🙂

@darwin Planck may be ready for Canary now

great, let me add it as a second task and test it

do you want to run full matrix or want only specific build?

I think a specific build is sufficient for these purposes. You could pick Linux, clang

How will you trigger a Planck build? Do you need a token from me?

@mfikes one last thing, will need CANARY_PLANCK_TRAVIS_TOKEN encrypted in https://github.com/cljs-oss/canary/tree/jobs

.travis.yml

travis encrypt CANARY_REPO_TOKEN=xxx -a env.global

btw. it is nice that you don’t have to tell me, you will encrypt it for travis eyes only

So, I’ll get a token, substitute it in for xxx above, and then what do I do with the result?

change also the env var name

and run it in jobs checkout, it will modify the .travis.yml file

then commit

ideally with [ci skip] so you don’t trigger canary

look at some other “config” commits in that branch

@darwin By “change also the env var name”, should I change CANARY_REPO_TOKEN in the command above to, perhaps PLANCK_REPO_TOKEN?

to CANARY_PLANCK_TRAVIS_TOKEN

ok

OK, so it indicated

Detected repository as mfikes/planck, is this correct?

that’s wrong, you have to checkout canary jobs branch and do it there

this is for canary’s .travis.yml

Ahh PEBKAC

@darwin It is wanting to make changes that involve stripping off quotes. I’ll make a gist…

maybe you have some other travis cli tool version than I do? they also try to reformat .travis.yml

@darwin https://gist.github.com/mfikes/aba14f3336157fbb002176e872632d08

mine is 1.8.8

$ travis --version
1.8.8

hmm, that stripping of quotes look ok to me

I’m on macOS

me too, feel free to commit it

OK

add [ci skip] into commit message

Just [ci skip] or do I need to add jobs info?

“add CANARY_PLANCK_TRAVIS_TOKEN into env vars [ci skip]”

this won’t trigger canary job

this is just a configuration tweak

My commit message should literally be that? Without the quotes?

yes

Done

@mfikes FWIW [ci skip] or [skip ci] is a Travis thing, not canary

Yep.

I wondered a bit why CLJS-2245 was necessary, because using local node modules worked before, but I see CLJS-2240 changed code so that node modules were indexed only when :npm-deps is given

now when you are at ti, you may try to trigger a new canary job

git commit -m "job --compiler-repo --compiler-rev canary" --allow-empty

OK

Change anything there to point at Planck, or literally that?

@juhoteperi right this is about making it explicit. It would shell out even if you didn’t want it to 😛

@mfikes I’ve added it here https://github.com/cljs-oss/canary/commit/38315c6056067d28e8898d620e5e2ba8f9a58433

but it will build whole matrix now

haven’t figured out how to exclude some configs from matrix

will probably overwrite your compiler and os config keys without studying matrix.include stuff

I don’t see any Travis jobs running. Is this supposed to trigger one?

@anmonteiro Hmm, it previously would only shell out to node to resolve dep graph, not npm

you have to push the commit

I pushed the first commit. But I’ve not done

git commit -m "job --compiler-repo  --compiler-rev canary" --allow-empty

@mfikes yes

But it is fine it is now explicit. Just wanted to be sure to understand the changes

oh hrm you’re right

I might have missed that

I think we’ll have the same behavior if we revert both CLJS-2240 and CLJS-2245

because it detects node-required anyway

@mfikes I’m not sure if clojurescript master would work now, did the testing on my fork with some patches applied, for master it would then be just git commit -m "job" --allow-empty or git commit -m "job -r SHA" --allow-empty

@anmonteiro lein test failed on my ClojureScript CI https://travis-ci.org/mfikes/clojurescript/builds/253989288

lol

I forgot to commit an untracked directory

nice catch

I’ll fix in CLJS-2246

Sweet! Canary is building Plank now 🙂

@mfikes the main job already finished with this report: https://github.com/cljs-oss/canary/tree/results/reports/job-000044-1.9.807-f63ab74 btw. canary just triggers the travis builds but does not wait for them, that is probably the last major thing to implement

Cool!

and btw. I realized we cannot use http://shields.io badges or travis badges, they show just most recent build status

at least I didn’t find a service which would provide icons for specific builds

@juhoteperi nice catch. let me know if this looks good: https://dev.clojure.org/jira/browse/CLJS-2246

Planck build passed. 🙂 Will try with master now.

@mfikes ok, go for it 🙂

Great work @darwin !

thx! 🙂

@mfikes finished https://github.com/cljs-oss/canary/tree/results/reports/job-000045-1.9.806-a0ed8c4 btw. the output is more verbose: https://travis-ci.org/cljs-oss/canary/builds/254002712 that was killing travis in case of fresh caches

docker image is cached now, so it worked

This is so cool!

Canary built ClojureScript master, put a JAR at a URL that other repos can get, and triggered Planck to buiild with that JAR.

@mfikes didn’t find a way how to limit travis api request to launch only specific build from matrix, their docs are not very specific about this: https://docs.travis-ci.com/user/triggering-builds and google didn’t help, tried this https://github.com/cljs-oss/canary/commit/67ff99da0c5da9e711bbab7c0edf47c75b8fcd0f but had to revert it back, it didn’t work

OK, either way is fine.

let’s leave this open for now, it is not that important to be solved ATM

cool

@anmonteiro lein test has an error for me on master, specifically test-node-modules-cljs-2246 has an exception

@dnolen you need to install yarn

or we could revise the test to not use yarn I guess

actually that might be better

yeah I don’t think the test should rely on yarn

fixing

there’s enough requirements as is

@dnolen https://dev.clojure.org/jira/browse/CLJS-2248

great thanks

on a plane to Berlin now 🙂

oh wow

getting some time to hang before the conf

👍

will try, doing some Cognitect client work before the conf

@anmonteiro tests still failing for me after the patch

passing locally, what did I do wrong

CI also passes

hrm

do you have a package-lock.json file or something?

do not but I had a package.json

something that may be preventing module-deps from being installed

trying again after deleting package.json will paste what I see if it fails again

thanks

(hoping to QA :global-exports at the REPL after I get past this)

just re-ran lein test again and no failures

already noticed some other broken things that need tests (:foreign-lib local overrides don’t appear to work anymore)

and I don’t have any untracked files

@anmonteiro still not working for me

so odd

lein test :only cljs.build-api-tests/test-node-modules-cljs-2246

^ does that succeed?

trying that - removed any random testing stuff first

@anmonteiro running just that one worked

@dnolen ls $TMPDIR | grep 'test-out$' | xargs rm -rf

then lein test again? 🙂

trying that

well that’s not deleting the folders for me

I might have given you a botched command

that’s ok I think I cleaned it up - trying lein test again

this is the actual command fwiw: cd $TMPDIR && ls $TMPDIR | grep 'test-out$' | xargs rm -rf

lein test still doesn’t work for me after manual clean

trying your command now

no change

do you have a dirty worktree?

I’m out of ideas here

the patch applied, right?

not other than your patch

@dnolen can you push it to master and let Mike’s CI run it?

well I want to make sure I can get back to a sensible state too

node -v
npm -v

can you run those so I can try to repro?

v6.11.0

3.10.10

thanks

master is just broken for me now

and the patch is just a different broken thing

I’m on 8.1.4 and NPM 5

so that’s probably why

installing those

I’ll modify the patch to make CI run Node 6 as well

so we can prevent further failures

I need to get master back to sensible state for myself - so first I’m probably going to comment out 2246 test

and push that until this can get sorted out so I can focus on other things

OK reproed your failure

feel free to comment that test

I’ll rebase on top of master when I figure this out

I think CI running the the lower bound of Node.js releases is generally a good idea

not really interested in bleeding edge Node.js stuff other than making sure it also isn’t breaking

makes me wonder how anybody gets anything done with that stuff

agreed we should be on LTS

which is 6 currently

tests appear to be passing if I comment out 2246, will confirm shortly

problem seems to be NPM

confirmed

because we call test/delete-node-modules

and it leaves empty directories

on the subsequent npm install NPM thinks the modules are installed

when really the folders are empty ¯\(ツ)/¯

heh

commented out the test on master

switching gears, but happy to try a patch if you come up with one

yeah

fixed now I think, running lein test

@dnolen replaced the patch with one that makes tests pass: https://dev.clojure.org/jira/browse/CLJS-2248

just ran lein test twice and couldn’t repro anymore

sorry to have made you waste so much time on this

also updated the Travis file to run Node 6

no worries, it’s going to take a while to get all this Node coolness under control 🙂

also waiting apply before the next release: https://dev.clojure.org/jira/browse/CLJS-2239

docstrings for :target :nodejs under self-host

to get the string-requires goodness

ok pushing up priority so I don’t forget

hrm I’m seeing a regression with :foreign-libs and :target :nodejs