Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support clojurescript projects #26

Open
viebel opened this issue Jun 30, 2015 · 19 comments
Open

Support clojurescript projects #26

viebel opened this issue Jun 30, 2015 · 19 comments
Assignees
Milestone

Comments

@viebel
Copy link

viebel commented Jun 30, 2015

I have tried to run lein yagni on a very small cljs project: https://github.com/viebel/cljs-explore
And nothing happened. The cljs files are not parsed.

@venantius
Copy link
Owner

I'm pushing this to the 0.2.0 release.

The first blocker is a clojure.tools.namespace problem - Yagni uses the project's namespace tracker to figure out which namespaces exist based on which files can be found in the source-paths, but http://dev.clojure.org/jira/browse/TNS-5?page=com.atlassian.jira.plugin.system.issuetabpanels:changehistory-tabpanel#issue-tabs indicates that it only targets .clj files, ignoring other filetypes in the .clj* universe.

There may be additional work to do after that, I don't know, but that's the first place to start.

@venantius venantius self-assigned this Jun 30, 2015
@viebel
Copy link
Author

viebel commented Jul 1, 2015

I have submitted a patch in Dec 14 regarding cljs support in clojure.tools.namespace. It has been rejected because "until a final decision has been made regarding Feature Expressions, which may include alternative file extensions."

Now that clojure supports reader conditionals, the patch might be approved

@venantius
Copy link
Owner

Hah, that's funny.

In any case, I'm fine building a direct workaround into Yagni in the meanwhile. I don't see a particularly compelling reason to delay implementing this until tools.namespace catches up to reader conditionals.

Only thing to note is that I personally do have something of a development schedule here. If you felt like taking it upon yourself to put together a PR I'd accept it, but otherwise I'm going to prioritize better JVM interop and tagged literals for the 0.1.x releases and delay implementation of this for 0.2.0

@viebel
Copy link
Author

viebel commented Jul 1, 2015

hmm.. I gave it a try but it's not easy: how could I make clojure.tools.namespace.track/load parse cljs files?

Meanwhile, I have renamed my cljs files to clj in order to make them parsed by yagni. But it didn't work at all. Running lein yagni is causing a compilation of the project. The compilation failed with:

Unable to resolve symbol: clj->js in this context, compiling:(cljs_explore/foo.clj:4:14).

So I removed the cljs specific code from my code. And then it works.

Also, lein yagni seems to trigger a running of the project. Do you know why?

@venantius
Copy link
Owner

The best way would probably be to refactor some reasonable section of that codebase out and just place it directly in Yagni; an alternative would be to use with-redefs or equivalent to just redefine the behavior of a single function.

The compilation error you're experiencing there is not being caused by Yagni, it's being caused by the fact that renaming the .cljs file to .clj means that you don't have access to clj->js anymore.

Yagni doesn't trigger a running of the project, but require-ing a namespace that has import side effects (as your sample project does) causes such side effects to evaluate.

@viebel
Copy link
Author

viebel commented Jul 2, 2015

  1. I'll give it a try later
  2. You mean that when we'll include cljs namespaces, this error will disappear?
  3. Be aware that cljs project almost always have side-effects. There is no main function called from outside. The code executes itself. So there will almost always be a namespace with side effects

@venantius
Copy link
Owner

  1. I'm doing a lot of work on the 0.1.2 branch right now, which is mostly addressing some of the problems Yagni currently has with JVM interoperability, and the more time I've spent there the more I suspect I'm likely to see a similar rabbit warren of compiler issues when it comes to ClojureScript. In particular, as I don't have as much experience with ClojureScript, I'm worried that resolving JS interop references may prove to be tricky, but that will just have to be a bridge that we cross when we come to it.
  2. Yes, it should. The error is a Clojure compiler error - nothing to do with Yagni. If you were to try to load that namespace into a Clojure REPL with the .clj extension you should see the same error.
  3. Hmm. There's some subtlety here; cljs projects targeting the Node runtime will have a main method, and others could take advantage of the sort of code layout in the Modern ClojureScript tutorial here, but it is likely the sort of thing we'd want to be explicit about in the README.
(defn init []
  (if (and js/document
           (.-getElementById js/document))
    (let [theForm (.getElementById js/document "shoppingForm")]
      (set! (.-onsubmit theForm) calculate))))

(set! (.-onload js/window) init)

@viebel
Copy link
Author

viebel commented Jul 28, 2015

look at this thread about cljs support in tools.namespace

@venantius
Copy link
Owner

Good catch, I'll comment in there.

@viebel
Copy link
Author

viebel commented Oct 2, 2015

any update about supporting cljs support

@venantius
Copy link
Owner

None, but you'll be happy to know that I'm now actively developing in ClojureScript on a daily basis, so I'll probably take a serious look at doing a port in the near future.

@viebel
Copy link
Author

viebel commented Oct 2, 2015

Great!
Happy cljs coding :)

On Fri, Oct 2, 2015 at 3:27 AM, Ursa americanus kermodei <
[email protected]> wrote:

None, but you'll be happy to know that I'm now actively developing in
ClojureScript on a daily basis, so I'll probably take a serious look at
doing a port in the near future.


Reply to this email directly or view it on GitHub
#26 (comment).

"Are we what we become or do we become what we are?" - Prof. Beno Gross

@arichiardi
Copy link

This would be a great addition given that eastwood does not support ClojureScript. Any update/things to do?

@venantius
Copy link
Owner

At the moment I'm afraid my attentions are focused elsewhere. I'd happily take a pull request if someone felt up to the challenge.

@vemv
Copy link

vemv commented Feb 14, 2017

Still no luck?

Cheers - Victor

@venantius
Copy link
Owner

venantius commented Feb 14, 2017

I'm afraid Yagni is basically unmaintained at the moment. My day job right now doesn't involve me writing Clojure and while I use some of my Clojure tooling with frequency (e.g. Ultra, my vim plugins) I'm not using Yagni at all. If I were going to return to this project I would probably consider a comprehensive rewrite.

@venantius
Copy link
Owner

That having been said, I'm more than open to a PR.

@vemv
Copy link

vemv commented Feb 14, 2017

Thanks for taking the time to answer - much appreciated!

@st4cks1defl0w
Copy link

If anyone is interested, I wrote a complementary tool for cljs https://github.com/stacksideflow/cljs-yagni (implementation-wise this tool is very different though, based on dynamic analysis and source-maps), tested on some biggish codebases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants