@tony.kay thought about a bit and I think the implementation should be completely separate from the “API”

https://github.com/thheller/cljs-i18n-api

the API just covers the tr macro and extracting the strings into the analyzer data

your fulcro.i18n could provide a ITranslate implementation at runtime

also enables tools like shadow-cljs to do everything completely transparent

could maybe become part of CLJS itself since the API surface is very minimal

but really doesn’t have to be so probably shouldn’t

might be beneficial to have a generic i18n API for CLJS though

oh I also rolled the trc,trf,tr macros into just tr. seems like an easier API

@thheller I'll look it over. Surface reaction is that from a technical standpoint it seems solid. From a practical standpoint I need to think more about it. Not sure about the API change. The existing API is more clear I think, and it is more amenable to existing tooling which eases transition (e.g. what if only shadow-cljs implements your suggestion...then the "normal" cljs users lose i18n?

The newer trf accepts named parameters or a single map...the named is for bw compat. Using a map turns out to be more practically useful.

and given the map preference, a change in API to one call might better be:

(tr msgid {:cljs.i18n/context "optional context" ...other params...})

context is rare, and ns aliasing makes it short to use

that also makes the API easily extensible: just add more nsed kws to the options map

that also makes it possible to pass additional info and custom options through to the string extraction layer.

well...as literals at least..wouldn't want to deal with the resolve issues probably

and I guess as far as API goes: trc and trf of Fulcro are just macros on top of this new macro...so bw compat is easy

I like it

does shadow have an existing hook point to use this today? Or would it require new hooks?

@tony.kay hmm I like the map instead of kw-args but what if the map is completely dynamic? we can’t extract :cljs.i18n/context then?

nothing in shadow-cljs yet, want to sort out the API first

can’t really call this cljs.i18n without some sort of approval that this is ok since I consider cljs. sort of reserved

So, I guess the options are: - Have a tr with the optional vector notation for context - Have trc to supply context - Make the macro error check and disallow variables…actually the current tr does that, since it has to be a literal string or we can’t extract it. There is a tr-unsafe variant that is unchecked…because sometimes you do need to use a variable for the string.

could use a map instead of the vector too

can add a tr-unsafe fn to the cljs.i18n ns that just delegates to the ITranslate protocol as is

yeah…it’s just sanity checking at compile time

but thinking about it .. no matter what the :cljs.i18n/context is better than the vector notation

(tr msgid {:cljs.i18n/context "optional context" ...other params...})

(tr msgid :cljs.i18n/context "optional context" ...other params...)

works both ways

Just throwing this out there: ` (tr “message”) (tr “context” “message”) (tr “message {a}” {:a 1}) (tr “context” “message {a}” {:a 1})

` why use a vector or map

I don’t particular like the extra string for context

they have to be literal strings, so they are easy to detect and parse

to me it feels like I always want the string first, then the context?

but yeah easy to detect

perhaps the keyword is better though…it is extensible

APIs are such a pain to get right 😉

yeah. I kinda feel like the “wrapped” map argument may encourage too much dynamisn

and we need this info to be static

(let [args {:foo "bar"}] (tr "translate this {foo}" args)) doesn’t work?

Yeah, I understand the problem wit macros….I still lean towards the separate macro name for context

oh, that does work

(your example)…the args pass through

well we dont NEED the args to be static but it feels more explicit and automatically error checks?

in fact, the args need to be dyanmic

for formatted stuff

the values need to be dynamic? the keys don’t?

in formatted stuff, allowing a dynamic map to fill in params is a requirement of the API

for sure

well, pretty for-sure 🙂

facilitates writing things compositionally

it would be more convenient yes but for error-checking reasons static sounds more reasonable

composition goes out the window anyways since this is a macro 😉

yeah, but an error means what? It doesn’t format correctly…I think they’ll see that

I mean thing like (tr "hello {name}" :nane username) which could check if the map contains all the required keys

(defn formatted-user-info [args]
   (str (tr "..." args) (when (some-cond? args) (tr "..." args)))

kind of things

typo detection basically

I understand the error checking…and I don’t care about that error checking because it breaks the more interesting and useful cases I do care about

if the message doesn’t format correctly, they see that in the output…no compile check needed

If they fail to use a literal string, then extraction fails…and that would be hard to catch without compiler help…so I do care about that one

it’s about which errors matter

(defn formatted-user-info [{:keys [name condition suffix] :as args}]
  (str (tr "..." :name name ) (when (some-cond? condition) (tr "..." :suffix suffix)))

more to type yes but easier to verify 🙂

but are you going to parse the format string?

and support the evolving FormatJS format string ICU format?

done at compile time, so why not?

including plurals

and that also kills pluggable i18n libraries

I feel like Edna from The Incredibles…“No Capes!”

hehe

I think it adds a lot of complexity, a little benefit

I guess thats true

and I like the fact that the philosophy of maps in clojure is “I can carry extra stuff and things shouldn’t care”

so, if I have a “user” defrecord and want to use it as args, why shouldn’t I?

(defn formatted-user-info [args]
  (str (tr "... {some-key}..." args) (when (some-cond? args) (tr "... {other-key} ..." args)))

also kinda captures what args is expected to contain

true

it does, and FormatJS et al will check that

> if I have a “user” defrecord and want to use it as args, why shouldn’t I?

yeah, having to type that out again would definitely suck

although not sure a defrecord is going to work 😜

it would

do those convert to js properly?

so, back to context….metadata?

nah…that’s only boolean convenient

could always treat the last arg as a map on uneven arg count

(tr "..." :cljs.i18n/context "foo" args)

I really prefer if there was just a trc

(tr "..." :cljs.i18n/context "foo" {:key value})

it’s the shorted thing to type, is trivial to resolve, easy to read

(tr "..." :cljs.i18n/context "foo" :key some-value)

but not extensible?

well trc could also allow a map as context maybe? (trc {} "..." ...)

not sure, something bothers me about having the context first

ok, so what if we allow zero-or-more k-v pairs to give extensible options?

then trc et al are just wrappers

too bad we can’t have metadata on strings 😞

(tr msgid k-v-pairs map-of-args)

yeah, I thought that too

no named params for args

trf could be a wrapper that allows args as named params

thus, the core API is just tr…everything else is optional wrapper

the macro doesn’t get much more complicated when we allow kvpairs instead of a map

hard to detect which are options and which are args

so not a fan of an extra trf

nah, uneven args the last one is expected to be a map

oh, and just roll context et al into args since you don’t know for sure where the boundary is? I guess that’s ok 🙂

yeah

not suggesting trf is in the core lib…but Fulcro (and other consumers) could provide it

yeah the fulcro api doesn’t need to change at all

just emit calls to the cljs.i18n instead of doing anything itself

yep

I was thinking through both user API and implementation…co-mingling…separating them back out now

will need a proof of concept impl anyways and fulcro pretty much already has everything in place

the only point of the cljs.i18n API is pretty much to have something that build tools can access later

its fine if libs wrap it

OK, I’m in 🙂

ok, I’ll update the code/readme

and start the impl for shadow-cljs

let me know what I can do

I can take a first draft at making Fulcro’s lein template support shadow-cljs

right, I wanted to look into that as well 😛

if you wanted to give me any quick tips, I’m glad to refine/test it

shadow-cljs currently doesn’t allow a custom cljs.user, thats one things I saw in the template

but you do have an after hot load…it’s mainly used to re-call mount

yes :devtools {:before-load ... :after-load ...} both taking fully qualified symbols

:after-load would be figwheels :on-jsreload or whatever its called

so, to separate code for “just dev” you just make sure not to require it from prod code, right?

yep

“dev” code usually is loaded via the :preloads option

so the cljs.user namespace you have currently would be a preload

nothing forbidden about the name?

i.e. it would be a preload…but does the ns need to be something else?

I pretty much always encourage this pattern

https://github.com/shadow-cljs/quickstart-browser/blob/master/src/starter/browser.cljs

ok, I’ll start with that

init is called from the HTML (once)

hot reload calls stop, then start

same deal for production

since no code ever calls stop it will be removed

if everything is set up properly no dev code will be left alive

Dynamic module loading…same loader API from main cljs?

hmmm…seems it is different? https://github.com/thheller/shadow-cljs/wiki/ClojureScript-for-the-browser#module-loader

I use the loader for dynamic routing and locale loading

yeah cljs.loader is not great IMHO. it completely does not work with :module-hash-names which hashes output files for caching purposed

so instead of a main.js you get main.<some-hash>.js

since the cljs.loader does things on compile that doesn’t work

I’m not in love with it either, but does it work without the hashed names?

can’t know the md5 before finishing compilation

I added rough support for cljs.loader so if you use that it should just work

just ignore :module-loader

:module-loader existed about a year before cljs.loader so it isn’t built on top of it

not sure if I can rework it to just use that due to the ^:const injection stuff. makes it really icky to use

also not a fan of the manual cljs.loader/set-loaded! calls

and requirement that the “entry” namespaces requires cljs.loader

um…there was something about the non-manual set-loaded! they tried that didn’t work. It bit me in particular

all of that is handled transpartenly by shadow.loader

I know it was broken the way it was done for cljs.loader yeah

but not aware of any issues for shadow.loader

doubt there are many users though

cool. well, it probably would not be hard to support shadow’s through dynamic resolution

(of either I guess)

it would be better if cljs.loader just provided the API and didn’t enforce an implementation as well 😞

yeah, that seems less than optimal

anyways, just stick with cljs.loader and it should work as in CLJS

ok. Thanks for the tips. I’ll let you know how it goes. The dynamic module loading is a core part of dynamic routing for code splitting and i18n locales

thinking about it some more it might be practical to just create a spec for the analyzer data

and let macros put it there however they want

Sure. Sounds reasonable. since you’re still around…trying to run CI tests from command line. Seems that is TBD?

running the tests is TBD yes. generating you can do via the :karma target

https://github.com/thheller/shadow-cljs/issues/57#issuecomment-356226346

oh, and then run a karma config on the output?

yep

I’ll give that a shot

only did very basic work on that but I think @mitchelkuijpers is using it

it’s really just for CI…so anything that makes them run works for me 🙂

basically it just outputs a single file that karma understands

more i18n fun, evidently. I got the stuff to compile with the stuff you and Mitchel had talked about with the intl-messageformat, but when I try the tests it dies on this line of generated js:

shadow.js.shim.module$intl_messageformat = require("intl-messageformat");

whats your config?

:ci-tests {:target :karma :output-to “target/ci.js” :ns-regexp “^boo\\..*-spec$” :modules {:main {:entries [boo.CI-runner]}}}

:modules you don’t need, the runner is also auto generated

can set :runner-ns boo.CI-runner if you want though

good to know…trying it with just regex

not sure why its emitting require

checking

well, that ns would have pulled in intl stuff

same problem after a clean build

hmm try settings :js-options {:js-provider :shadow}

not sure why its picking require, really shouldn’t

just FYI, that code is generated when compiling i18n.cljc from Fulcro

doesn’t matter

k

js-options goes at the same level as target?

yep

https://code.thheller.com/blog/shadow-cljs/2017/09/15/js-dependencies-going-forward.html#choosing-a-provider

covers what :js-provider means, you are on :require which is only supposed to be used for node builds

I see

that fixed it

you want me to make a commit of this little project so you can diagnose?

no, I just forgot setting it

ah, easy fix then 🙂

I’ve got most of it working…the CI thing has been the only hard part so far

maybe commit what you have so I can look over it and make suggestions

https://github.com/awkay/shadow-template

need to make karma auto-exit with a status code still…

the default runner does that

you really do not need the boo.CI-runner

I thought I remved it

oh right…the file

(enable-console-print!) is also never required anywhere

shadow-cljs injects that once in the proper place

great

you don’t need https://github.com/awkay/shadow-template/blob/master/src/test/boo/tests_to_run.cljs if the use the :karma stuff

the :karma finds all namespaces that matches your regexp and makes sure it is loaded before the :runner-ns

so no manual upkeep with those tests-to-run files, although you can of course still do it manually if you prefer

I still need to require them for the spec main I think

for live test running of Fulcro Spec

shouldn’t need it no

https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/test/karma.cljs

During development hot code reload is used to trigger test re-render, and all the tests have to be present in the browser or it won’t find them

NOT karma

Fulcro Spec

https://github.com/awkay/shadow-template/blob/master/shadow-cljs.edn#L15

right .. but … I “hack” into cljs.test and register all tests so they are available at runtime

oh, I see

ok

cool

so you do not need to damn macro to know which tests exist

so what should that build look like then? I have to point it at something

but yeah if you stick to the default cljs.test/run-all-tests that doesn’t buy you anything

with a :browser target it doesn’t know to scan for tests, and without the module thing it wouldn’t run the spec rendering trigger

:browser-test is meant to that

oh, I see

https://github.com/thheller/shadow-cljs/issues/57#issuecomment-346464244

I missed that…that should work

so you could create a :runner-ns fulcro.shadow-test-runner or so

pretty simple https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/test/browser.cljs

thats also re-usable and doesn’t need to be in the users project

since it doesn’t rely on a macro call

the shadow.test is basically a macro-less copy of cljs.test

https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/test/env.cljs lets you access all deftest

Unfortunately Fulcro Spec defines it’s own thing internally

right .. you don’t have to use shadow.test (or cljs.test)

the point is that the :browser-test target will ensure that all namespaces matching the regexp will be compiled and loaded before the runner

what the runner does is completely up to you

Yeah, that’s all I need

It was working as I wrote it, but that will be cleaner

if you have everything setup so the tests are registered at runtime you don’t need to go the macro route that cljs.test took

The extra compiler magic of scanning/loading is a nice additional touch

there is probably a whole lot more build tools could provide if properly enabled to do so

:test     {:target     :browser-test
                   :output-dir "resources/public/tests"
                   :asset-path "/tests"
                   :devtools   {:after-load boo.client-test-main/start
                                :before-load boo.client-test-main/stop
                                :preloads   [devtools.preload]
                                :http-root  "resources/public"
                                :http-port  8021}}

oh…output-to

just set :test-dir

https://github.com/thheller/shadow-cljs/blob/master/shadow-cljs.edn#L131-L137

the rest should be inferred

unfortunately :http-root currently has to be static part of the configuration so you still need to repeat that

ideally that would be inferred as well

also set :runner-ns

that will automatically set :after-load <runner-ns>/start (and stop)

so I need to serve an HTML file somewhere in here

it generates one currently

Spec needs a div to mount onto

I’m beginning to think what I had was better in some ways…I just want the scanning. The rest of the magic is distracting

you can create the :test-dir/index.html manually

ok

let me try that…

https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/build/targets/browser_test.clj#L23-L33

thats the default that gets generated otherwise

but yeah the test stuff is pretty rough

open to any kind of suggestions

I’m convinced that we can probably get a testing setup without any configuration at all

that just works

eg. shadow-cljs clj-run fulcro.spec.server

pretty easy for CI…would add a dep on Fulcro Spec for browser I would think

not really…you just generate the code…yeah, that is probably true

the hard part is running the tests

generating some code is easy

not sure how much shadow-cljs should help with running them, its a build tool after all

but in case of :browser-test we can get close enough to something usable

with some “convention over configuration” we can remove most config config options

well, at least for Fulcro Spec it was built around “it works if you have hot code reload”…no other support was assumed, because there wasn’t any 🙂

yeah but you also generate a whole bunch of files in the template that may confuse users

I prefer generating as little files as possible. also less likely for updates to break stuff just because a generated file is now outdated

so if the build tool can offload some of the generation that can be very useful

:test     {:target    :browser-test
                   :test-dir  "resources/public/test"
                   :ns-regexp "^boo\\..*-spec$"
                   :runner-ns boo.client-test-main
                   :devtools  {:http-port 8021
                               :preloads  [devtools.preload]
                               :http-root "out/demo-test-dummy"}}

ooops

:preloads [devtools.preload] is no longer required as of [email protected]. it is automatically added if cljs-devtools is on the classpath

http-root 😕

right hehe the directory

:ns-regexp "-spec$" is also safe to do since it will only include matching files, eg. nothing in .jar files

who wants to run other peoples tests 😉

but looks good otherwise

/js/test.js is the generated file in case you are using a custom index.html

yeah…working on that now. It’s up, just going to check the hot reload

looks good

I need to push spec’s css into component-local css. I wrote the very early draft of spec, but someone else wrote the bulk of it…it needs some work…

the :cards build may also benefit from the :browser-test treatment?

auto-scanning for devcards…yeah

I don’t really recommend running via lein but its ok I guess

still need to add that tools.deps support

well, I need the server support stuff in project.clj, and Cursive…

don’t want to manage two deps lists

(three, really, with npm)

yeah understandable

you can :lein {:profile "+cljs"} and create a dedicated :cljs profile in project.clj if desirable

technically shadow-cljs should just be a dev dependency

yeah, I haven’t (obviously) finished with the project file

still have all the cljs builds defined

just wanted to mention the option of :lein :profile 😉

yeah, I’ll probably add that. ANy clue why Karma won’t exit, and then disconnects? I’m calling the done from stop

or am I?

ah, no…that is all your code

perhaps I’m missing something

hmm I though karma always keeps running?

for CI it needs to exit with a status code

when I was using doo it managed it, so I don’t know

Ah…I have a very old version of the karma stuff

I never went beyond running karma start which re-runs tests on file change so it isn’t supposed to exit

singleRun is supposed to exit so that you can use it with things like Travis and CircleCI

right

npx karma start --single-run
11 01 2018 00:07:03.055:INFO [karma]: Karma v2.0.0 server started at 
11 01 2018 00:07:03.058:INFO [launcher]: Launching browser ChromeHeadless with unlimited concurrency
11 01 2018 00:07:03.063:INFO [launcher]: Starting browser ChromeHeadless
11 01 2018 00:07:03.314:INFO [HeadlessChrome 0.0.0 (Mac OS X 10.12.6)]: Connected on socket c0LoCJjPKzBp3pRIAAAA with id 16697457
LOG: 'Testing shadow.inspect-test'
LOG: '{:type :kv, :type-desc "cljs.core/PersistentArrayMap", :count 1}
'
LOG: '{:type :coll, :type-desc "cljs.core/PersistentHashSet"}
'
LOG: 'Testing demo.app-test'
LOG: 'once before'
LOG: 'each before'
HeadlessChrome 0.0.0 (Mac OS X 10.12.6) demo.app-test a-failing-test FAILED
	FAIL in   (a-failing-test) (at http:60704:85)
	expected: (=
	            1
	            2)
	  actual: (=
	            1
	            2)
	    diff: - 1
	          + 2

LOG: 'each after'
LOG: 'each before'
LOG: 'each after'
LOG: 'once after'
LOG: '
Ran 3 tests containing 1 assertions.'
LOG: '1 failures, 0 errors.'
HeadlessChrome 0.0.0 (Mac OS X 10.12.6): Executed 3 of 3 (1 FAILED) (0 secs / 0.011 secs)
HeadlessChrome 0.0.0 (Mac OS X 10.12.6) ERROR
  Disconnected, because no message in 10000 ms.
HeadlessChrome 0.0.0 (Mac OS X 10.12.6): Executed 3 of 3 (1 FAILED) DISCONNECTED (10.03 secs / 0.011 secs)

guess I’m supposed to call something

it does exit with 1 though

right, but it also does so when they pass 😉

because of the disconnect

ah hehe makes sense

let me check what I’m supposed to be calling

I’m going to try to upgrade my deps

yeah, single-run just isn’t working

k I think I found it

my brain seems to now work anymore

(prn :calling-end-run-tests)
                (ct/report {:type :end-run-tests})
                (prn :done-end-run-tests)

(defmethod ct/report [::karma :end-run-tests] [_]
  (prn :in-end-run-tests)
  (js/__karma__.complete #js {"coverage" (aget js/window "__coverage__")}))

(prn :in-end-run-tests) is never printed

the other two are

mystery to me why

wrong multimethod dispatch key?

indeed

::karma getting the right ns?

the env seems to be cleared prematurely

indeed

should be fixed in [email protected]

I’ll try it out

cooking some food…sorry for the slowness

perfect!