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

Advanced compilation breaks mixins with custom methods #29

Open
loganlinn opened this issue Jul 31, 2014 · 8 comments
Open

Advanced compilation breaks mixins with custom methods #29

loganlinn opened this issue Jul 31, 2014 · 8 comments
Labels

Comments

@loganlinn
Copy link
Member

Reported here: https://groups.google.com/forum/#!topic/clojurescript/YVPnwIWaUVU

If a mixin has non-lifecycle methods (like the set-interval example), advanced compilation breaks invocations of those methods. Was able to reproduce on the mixin example with these changes.

Call-site of undefined is not a function:

return $output_schema13868$$1$$.$owner$.$set_interval$(function() {

Mixin is defined here: (note that it's getting inlined)

$obj__7119__auto__$$inline_244$$.mixins = [{set_interval:function($f13846$$, $t13847$$) {
  return function() {
    return function($f13846$$, $t13847$$, $t$$16$$) {
      return $f13846$$.$intervals$.push(setInterval($t13847$$, $t$$16$$));
    };
  }(this).call(null, this, $f13846$$, $t13847$$);
}, componentWillUnmount:function() {
  return function() {
    return function($owner$$30$$) {
      return $owner$$30$$.$intervals$.map(clearInterval);
    };
  }(this).call(null, this);
}, componentWillMount:function() {
  return function() {
    return function($owner$$31$$) {
      return $owner$$31$$.$intervals$ = [];
    };
  }(this).call(null, this);
}}];
@loganlinn loganlinn added the bug label Jul 31, 2014
@loganlinn
Copy link
Member Author

A (very ugly) workaround in the meantime is to use aget

(.set-interval owner #(swap! state update-in [:seconds] inc) 1000))
((aget owner "set-interval") #(swap! state update-in [:seconds] inc) 1000))

@sritchie
Copy link
Contributor

sritchie commented Aug 8, 2014

Yeah, I'm running into this too. This could probably be solved with a custom externs file - it does feel like something that we should be able to fix without resorting this, since the javascript's defined inline and should be handled by the cljs compiler, yeah?

@loganlinn
Copy link
Member Author

since the javascript's defined inline and should be handled by the cljs compiler, yeah?

Yeah. The mixin is currently represented as a literal JS object that's given to React. Something along the lines of,

var my_mixin = {"my_method": function() { ... }}

And it's not being associated with a call like,

(.my-method owner)

There was a fix in ClojureScript 0.0-2301 that might be relevant: clojure/clojurescript@d7c8960
However, I'm currently not able to pull that jar to test. But I'm not hopeful that is actually addressing this issue. I think this is a larger issue with Google Closure compiler and string vs literal accessors

@sritchie
Copy link
Contributor

Yeah, I don't think that new version helps. We're on the latest CLJS version and the issue's still up. Just wanted to let you know!

@swannodette
Copy link

There's really no way to make this work with advanced compilation. Using JS object literals with string keys like that simply won't work with regular method invocation. You need to provide an externs file.

@danielytics
Copy link

@swannodette Do you have any suggestions how this externs file would need to look? I'm struggling a bit how, for example set-interval would be defined so that the compiler knows that its part of the component.

Beyond that, while I'd prefer if it worked as is, I'm Ok with writing an externs file for the handful of mixins that I use. I just don't know how right now.

@loganlinn Would it be at all possible to change how the mixin is defined (not as a JS object literal) to side-step the issue? I'd be willing to try it and test, but I'd need some guidance as I don't really know where to start on such a thing.

@swannodette
Copy link

For the my-method case above you will need the following in your externs file:

Object.my_method = function() {};

@danielytics
Copy link

@swannodette 👍 Thanks!

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

No branches or pull requests

4 participants