ClojureScript

No support for prefix lists in this patch, since there is none for other libspec types.

By prefix lists, you mean [foo.bar Baz Woz ...] ?

This doesn't seem to work for me. E.g.:

(ns import-test
  (:import goog.async.Deferred))

(def ^:export d Deferred)

compiles to:

goog.provide("import_test");
import_test.d = import_test.Deferred;
goog.exportSymbol("import_test.d", import_test.d);

edit: Oh, but (Deferred.) does work. That's what I really want anyway. But the above would be nice, also for e.g.:

(extend-protocol Foo
  Deferred
  (foo [_]))

@David Nolen – Yes. I'm quite fond of prefix lists myself, but seeing how they're not supported for other libspec types... (Whenever the time is ripe for that to change, I'll be happy to provide a patch. )

@Tom Jack – Thanks, I'll look into it!

I guess the precise form of any new patch will need to depend on the resolution to CLJS-272.

Does this patch also handle importing deftypes & defrecords?

I wouldn't expect it to, since types & records are fundamentally different to GClosure classes in that they don't constitute separate "dependencies". For this reason e.g. (:require clojure.core.reducers.Cat) won't work, whereas (:require goog.string.StringBuffer) works fine.

It would be straightforward to have, say, (:import foo.bar/Quux) pull in Quux from the foo.bar ns. Of course that's already possible with :use / :require :refer, plus this kind of syntax would constitute a departure from the current state of affairs in Clojure; but if it seems worthwhile, I'll be happy to adjust the patch / provide a new one to support this.

Figuring out automagically whether a GClosure-style dependency or a native type is needed would be somewhat more involved, I think. I'll need to think about what exactly might be required.

Incidentally, I wonder if there's any chance of Clojure supporting deftype/defrecord in use/require... I'll think about this some more, see if I can find anything on the topic in clojure-dev archives and possibly bring it up and/or produce a patch.

In any event, I'll be attaching a new patch ready for application on current master. It does seem to address Tom's issue at least.

Some random thoughts re: how much of a problem treating records/types in :import otherwise than as in Clojure would be: we have {->Foo} and {map->Foo} for records; types are close to the metal, particularly perf-conscious users will already expect to be exposed to some platform details etc.

What do you think?

With (ns foo (:import goog.async.Deferred)), this does not seem to require goog.async.Deferred (so you get an error about missing 'Deferred' property on undefined).

Does the test pass because goog.string is required elsewhere?

After pondering this some more. I wonder if :import should simply be restricted to the Google namespaces? I don't really see another case for it. Related - a warning from :require about specs that look like the following:

(:require [goog.foo.Bar :as Bar])

Should be pretty simple to detect. It could suggest the correct :import.

@Tom: The test is indeed flawed since StringBuffer is being required by core, thanks for pointing this out! I need to use some Closure "class" which is available in the version of Closure library used by ClojureScript, but not required elsewhere.

Not sure what the story is re: g.a.Deferred. I understand you're seeing that behaviour in a project where bringing it in with a :require does work? At any rate, I'll be preparing a new version of the patch soon; I'll make double sure it works for me on some non-trivial case.

@David: I think convenience of working with GClosure classes is the no. 1 reason :import might be useful, so thus restricting it or introducing a restricted version first and only expanding it to cover types/records later seems ok to me. It occurred to me that types/records could be goog.provide'd in the JS output unless, say, goog.provide calls must occur early in the file, which I don't know to be or not to be the case – will check that later; if this turns out to work fine, we might as well support the full Clojure-like behaviour.

Re: warning, do you consider that :require form strictly incorrect? Is any :require spec of the form [a.b.Foo :as Foo] always incorrect (as opposed to not the most elegant way to go about it etc.)? Honest questions both.

This patch should actually work. The test now uses goog.math.Long and tests both constructor and "static method" calls.

Support for "static methods" is achieved by implicitly :require'ing the namespace/class :as the final segment of its name. Resolution of the ctor symbol is still handled by a separate :imports entry in the namespace map.

I haven't looked into implementing the type/record idea I mentioned before – that's coming next (if it works). Not sure if it should be a separate commit, an ammendment to the current one or a separate ticket.

Re: warnings, I think {:import foo.Bar} should cause foo.Bar to be :require'd anyway. If :require provided an option to :import – (:require [foo.bar.Quux :as Quux :import true]), say – the situation would be entirely analogous to that with :use / :refer (no strong opinion on whether that would be a good idea at this time). Plus there's nothing broken in (:require [foo.Bar :as Bar]); what is broken is going on to expect that (Bar. ...) will then be a ctor call. So maybe the warning should occur when (Bar. ...) is encountered in a namespace with this sort of :require spec? This sounds like a separate ticket though.

Here's a patch which causes emit :deftype*/:defrecord* to emit goog.provide when compiling a file. It does indeed allow :import to bring in cljs types/records.

Some notes:

1. This patch goes out of its way to prevent duplicate goog.provides, because (1) GClosure doesn't like them, (2) the test suite actually specifies (defrecord A ...) twice. I guess the sensible thing to do may be to fix the test to use unique record names and always emit goog.provide in files.

2. This could be suppressed in the presence of :private (or similar) metadata on the type/record name.

3. Assuming this is useful at all, it seems to me it might be better for this to remain in a separate commit and maybe a separate ticket.

Thoughts?

Wow, you really went for it!

What does the patch do to prevent duplicates? If this really solves the issues I see no need for a separate ticket. This brings us in line with Clojure :import semantics I'd say.

Oh if this patch works, it would still be nice to issue the warning I mentioned above as it will probably continue to be a source of user error.

There's a new dynamic Var in the patch, *emitted-provides*, with a root binding of nil and bound to (atom #{}) by compile-file* (in the binding form which handles *out*, *cljs-ns* etc.; *position* provides a precedent for binding an Atom to a Var). goog.provide calls are only output when *emitted-provides* is non-nil and @*emitted-provides* does not contain the currently processed symbol (which is then swap!'d in).

Stipulating that only one type/record can be defined with the given name in a given namespace would make the check around unnecessary, but then we should presumably check for duplicate type names and produce errors of our own (the GClosure error might be a tad cryptic from a user's POV).

In any case, outside of the dynamic extent of compile-file*'s bindings *emitted-provides* is nil and so no goog.provide calls are included in the compiled output (so compiling single forms for REPL interactions should work as it did previously).

As for the warning, I'm still not sure that :require form should be considered an error – it works fine and might even be what is meant if the user plans on defining a type under the same name. (The alias may be useful for calling "static methods" of the GClosure class.) That is in contrast to broken ctor calls which would work were a :require spec replaced with an :import spec; these are very likely to be errors (and otherwise they're probably caused by a missing declare), so we could warn when dealing with them (as outlined in my last-but-one comment here) – say, "bad ctor call, you might want to use :import rather than :require foo.Ctor" when dealing with a Ctor. ... form. Does this sound right?

@Tom – the new patch fixes a problem which would have prevented :import from pulling in g.a.Deferred correctly in situations where :require worked fine. Hopefully this was the only such problem (the new goog.math.Long test works fine, so this is likely to be the case). Thanks again for the heads-up!

I agree about the :require. Yes a warning about bad ctor calls would be nice - but only if the implementation isn't overly involved. Otherwise updating the documentation will suffice.

I also agree about producing our own warnings about only one type/record.

Ok. So we can just emit goog.provide always (as we do for ns forms) and disallow multiple deftype / defrecord calls within a single body of code delivered to GClosure. The attached patch does this (and adjusts the tests appropriately).

As for providing our own errors, the straightforward approach fails because for some reason deftype* forms get processed by parse 'deftype*. No idea why that is (and whether it happens for all such forms or only under special circumstances), will need to investigate. The GClosure error, on second glance, does include the full type name, so the user should be able to deduce what's wrong, so this patch needn't necessarily wait for a proper error check of our own.

Note that this is really about an error rather than a warning – if we only want to warn about duplicate types (and then handle them gracefully), then the original goog.provide patch is more appropriate (and we need to figure out how to warn the user / deal with the double parse).

(It would be easy enough to produce the error/warning at emit time rather than in the analyser, avoiding the double parse problem entirely... This just doesn't feel right though.)

Ok, so the last patch is broken in that it causes duplicate type names to become errors at the REPL. The original goog.provide patch doesn't suffer from this problem.

Given the above I think the original goog.provide patch may actually be the correct one to use.

I suppose it could be simplified to emit goog.provide when in compile-file* without regard to which types have been emitted, but that wouldn't be that much of a simplification and can wait for proper warning/error functionality (which I'm planning to figure out in any case).

The original :import-handling logic + the type/record support of the working patch rolled into one commit.

fixed, http://github.com/clojure/clojurescript/commit/0f37c9da05442293d0e6b2944c4c3b41aef52f87