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

compiler warnings in new cljs versions #144

Closed
thedavidmeister opened this issue Aug 22, 2017 · 7 comments
Closed

compiler warnings in new cljs versions #144

thedavidmeister opened this issue Aug 22, 2017 · 7 comments

Comments

@thedavidmeister
Copy link

       Aug 22, 2017 9:57:09 AM com.google.javascript.jscomp.LoggerErrorManager println
       WARNING: /app/.boot/cache/tmp/tmp/build_249b3a126e0f26fe96d0a5c79f919ce5/thedavidmeister-estimate-work-1535383fd184345db3dec0c34380e4682d8e6db5/fg/-b1ra1p/index.0f76fa787e214660aceb08719d8e4c33.html.out/garden/types.js:306: WARNING - Keywords and reserved words are not allowed as unquoted property names in older versions of JavaScript. If you are targeting newer versions of JavaScript, set the appropriate language_in option.
       return (!((other15033 == null))) && ((this15032__$1.constructor === other15033.constructor)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this15032__$1.function,other15033.function)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this15032__$1.args,other15033.args)) && (cljs.core._EQ_.cljs$core$IFn$_invoke$arity$2(this15032__$1.__extmap,other15033.__extmap));
@rap1ds
Copy link

rap1ds commented Aug 25, 2017

@thedavidmeister I saw the same warning today when updating ClojureScript from 1.9.292 to 1.9.908.

To fix the issue, set language-in and language-out compiler options to :ecmascript5.

;; project.clj

:cljsbuild
  {:builds 
    [{:id "dev"
      ;; ...
      :compiler {
                 ;; ...

                 :language-in     :ecmascript5
                 :language-out    :ecmascript5

                 ;; ...
                 }}

The default for :language-in and :language-out is :ecmascript3. Changing it to :ecmascript5 fixes the issue, but obviously that's not an option if you really need to target very old browsers, like IE8.

@noprompt
Copy link
Owner

@rap1ds @thedavidmeister Is this specifically a Garden issue? and, if so, can either of you provide a patch? @rap1ds it sounds like you might have some knowledge in this area. I'm very busy with work and family these days and I simply don't have the time to invest in researching this issue. Thanks!

@thedavidmeister
Copy link
Author

@noprompt garden is the only thing throwing this warning for me.

i don't exactly know, but i'm assuming it's the use of the name function (a reserved word in JS) here https://github.com/noprompt/garden/blob/master/src/garden/types.cljc#L6

@noprompt
Copy link
Owner

@thedavidmeister ugh, there's an old Jira ticket out there for this problem. It would appear the solution, for now, would be to rename that parameter to something else (funxion might be fun) and make sure that's handled anywhere else in the library.

@rap1ds
Copy link

rap1ds commented Sep 26, 2017

Would you consider the CSSFunction defrecord as a part of the Garden "public API"? I mean, if the name function is changed, is that then a breaking change? At least I'm using code like (:magnitude (px 1234)) in my app to access the pixel values (or am I doing it wrong? I didn't find any other way to access the value). So if someone is using the function the same way, the chande of the name would break the code.

However, changing the function name sounds like a good idea to me, even if it's a breaking change. In my own app I'm happy with changing the language settings to ecmascript5, but since ecmascript3 is the CLJS compiler default, it makes sense to fix the library so that it works without warnings with the default compiler options.

@noprompt
Copy link
Owner

@rap1ds Apologies for my delayed response. I think it's admissible to change the name here. It's a breaking change for sure. To the extent it happens to break someone upon upgrade, they can share their grievances here.

Feel free to submit a patch in response to this. I'm busy at the moment and can't get to it now.

danielcompton added a commit to danielcompton/garden that referenced this issue Feb 19, 2018
Newer version of the Closure compiler warn about naming a property
`function`. The CSSFunction defrecord has a field called `function`.
This commit renames it to `f` to avoid these compiler warnings.

This is a breaking change if anyone was relying on that field.

Fixes noprompt#144
@danielcompton
Copy link
Contributor

I've opened a PR to fix this in #158.

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

4 participants