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

how to deal with 'ReferenceError: Can't find variable: $' error #232

Closed
lorvent opened this issue May 13, 2016 · 29 comments
Closed

how to deal with 'ReferenceError: Can't find variable: $' error #232

lorvent opened this issue May 13, 2016 · 29 comments

Comments

@lorvent
Copy link

lorvent commented May 13, 2016

It was working fine for some files but now all of sudden, getting this error

ReferenceError: Can't find variable: $

how to fix it?

atleast if i know, on which file i am getting this error...i will be able to fix it.

thanks.

@RyanZim
Copy link
Member

RyanZim commented May 24, 2016

I'm guessing you forgot to include jQuery in one of your html files. You could try opening each page in the browser and looking at the console.

@lorvent
Copy link
Author

lorvent commented May 24, 2016

I don't think so because everypage uses many jquery plugins including bootstrap so that should not be the issue

but can you help me finding out more information?

like using plumber to get file name etc...

@RyanZim
Copy link
Member

RyanZim commented May 24, 2016

Not sure. Perhaps @XhmikosR, @mikelambert, or @ben-eb could help you better.

@XhmikosR
Copy link
Member

Unfortunately, this is an issue that happens for so long and personally I can't figure out why.

The solution is to either use an older uncss version (0.12.1 doesn't throw those errors), or stop inlining JS code.

@mikelambert
Copy link
Collaborator

I'll say the same thing I said in #220, if you want to give a relatively small reproducible test case, it's possible someone can look into it.

My guess as to the reason 0.12.1 doesn't throw these errors (this is for you too @XhmikosR) is that 0.21 just silently drops the warnings. This particular change was only included in 0.13: 62145b3

So you can downgrade, but I suspect you're really just hiding existing errors by doing so.

@lorvent
Copy link
Author

lorvent commented May 24, 2016

hmm nice tip but that doesn't serve real purpose as its just hiding errors,

whereas my whole idea of using build systems is to improve code quality...

@XhmikosR
Copy link
Member

Well in my case moving all JS to external files fixed the errors.

Plus all the advantages of doing so.

Ideally the issue should be fixed but I've yet to make a small test case.

I'll try to do it later or tomorrow.
On May 24, 2016 21:59, "lorvent" notifications@github.com wrote:

hmm nice tip but that doesn't serve real purpose as its just hiding errors,

whereas my whole idea of using build systems is to improve code quality...


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#232 (comment)

@jogaco
Copy link

jogaco commented Jul 30, 2016

Found the same issue.
This minimal example reproduces the error.

<!DOCTYPE html>
<html>
  <head>
    <script src="https://code.jquery.com/jquery-2.2.4.min.js" async="async"></script>
    <link rel="stylesheet" media="all" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" />

    <script>
      function a() {
        $('#div').show();
      }
      a();
    </script>
  </head>
  <body>
  </body>
</html>

@RyanZim
Copy link
Member

RyanZim commented Jul 30, 2016

Confirmed.

I'm getting ready to do a git bisect to find this bug.

@jogaco Thanks for posting!

@lorvent
Copy link
Author

lorvent commented Jul 30, 2016

in mycase, if you add js in html page we will get this problem,
move it to seperate js file

@RyanZim
Copy link
Member

RyanZim commented Jul 30, 2016

@lorvent Yes, moving JS to a separate file fixes it, but uncss should support inline JS.

I'm currently running git bisect to find which commit is responsible for this error.

@RyanZim
Copy link
Member

RyanZim commented Jul 30, 2016

And the results are:

62145b351fa9eedccdd8eeb5b8d5f14db72e4100 is the first bad commit
commit 62145b351fa9eedccdd8eeb5b8d5f14db72e4100
Author: Giacomo Martino <martino.giacomo@gmail.com>
Date:   Sun Mar 8 17:09:16 2015 +0100

    Output phantom.js errors to `stderr`.
    Use .contain

:040000 040000 812459bd91a7e73965bcadc7f0519a4c7fb21716 0e4e481eabfb8712f441e5ee469d0ed14ee88446 M  src
:040000 040000 21939bef4d3e7e6692b4725739e13591dc236480 88ee814adbd3125c385c4ec39232e8e18bedeea5 M  tests

@mikelambert You were right:

My guess as to the reason 0.12.1 doesn't throw these errors (this is for you too @XhmikosR) is that 0.21 just silently drops the warnings. This particular change was only included in 0.13: 62145b3

The first bad commit is 62145b3.

@RyanZim
Copy link
Member

RyanZim commented Jul 30, 2016

@XhmikosR Is this a PhantomJS bug? Perhaps ariya/phantomjs#14422 or ariya/phantomjs#14316?

@mikelambert
Copy link
Collaborator

The above HTML file uses an async sync, meaning the calling page will not block to wait for that script to load before loading/running the rest of the page. So when the call to $('#div') occurs, there is no guarantee that the jquery code has been loaded yet. And if the jquery code has not been loaded, I would expect you to get an error very similar to Can't find variable: $.

So from my perspective, the above HTML file is broken code, and uncss/phantom is just errorring on the broken code.

@XhmikosR
Copy link
Member

That is correct.

my example in #220 has a snippet which doesn't use async that triggers the
error though.

On Jul 30, 2016 11:26 PM, "Mike Lambert" notifications@github.com wrote:

The above HTML file uses an async sync, meaning the calling page will not
block to wait for that script to load before loading/running the rest of
the page. So when the call to $('#div') occurs, there is no guarantee
that the jquery code has been loaded yet. And if the jquery code has not
been loaded, I would expect you to get an error very similar to Can't
find variable: $.

So from my perspective, the above HTML file is broken code, and
uncss/phantom is just errorring on the broken code.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#232 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAVVtS-XI-PBWUbDHJ4J7zwVoGYx4lbqks5qa7ONgaJpZM4IdoGz
.

@mikelambert
Copy link
Collaborator

Well the minimal repro example above has a bug, which uncss found. Your example in #220 is probably due to a different issue then (though due to the same issue of a jquery file not being loaded properly). I'll try to respond with some ideas in that thread.

@jogaco
Copy link

jogaco commented Jul 31, 2016

What about this?

<!DOCTYPE html>
<html>
  <head>
    <script src="https://code.jquery.com/jquery-2.2.4.min.js" async="async"></script>
    <link rel="stylesheet" media="all" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" />
    <script>
   $(document).ready(function(){
      function a() {
        $('#div').show();
      }
      a();
    });
    </script>
  </head>
  <body>
  </body>
</html>

@XhmikosR
Copy link
Member

You are still missing the point. You are using async.

@mikelambert
Copy link
Collaborator

You are still referencing the $ variable before jQuery is loaded, so you will still get that error. You can either write code that doesn't use $ until after the page is loaded (which your example tried but did not do), or you can stop using async (and ideally move your <script> tags out of your )

@jogaco
Copy link

jogaco commented Aug 1, 2016

Got it!

@RyanZim
Copy link
Member

RyanZim commented Aug 1, 2016

Oops, didn't look at the test case close enough.

@RyanZim
Copy link
Member

RyanZim commented Aug 7, 2017

Can anyone check if this is fixed with uncss 0.15.0? We switched to jsdom in this release.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

Doesn't seem to throw even with async for the above sample #232 (comment).

But the test case from #220 (comment) still throws because there the file is local.

I even tried with --htmlroot in the cli and still can't find the local jquery file.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

Err scratch that. It works fine. I need to sleep :/

@XhmikosR XhmikosR closed this as completed Aug 7, 2017
@XhmikosR
Copy link
Member

XhmikosR commented Aug 7, 2017

Although, it still misses the JavaScript added selectors. This is still an issue, right?

@mikelambert
Copy link
Collaborator

I'm not sure what you mean by 'js added selectors'. I don't see that mentioned in this issue?

If you're talking about classes that get added to the page at runtime in response to user events, then yes, it misses those. I consider this a "Will Not Fix" bug though.

@XhmikosR
Copy link
Member

XhmikosR commented Aug 8, 2017

Why is that? It's a pretty common use case and almost every single website does this.

On a side note if you try my example from #220 (comment) with the latest version, it even misses the inline styles.

<!doctype html>
<html lang="en">
<head>
    <meta charset="utf-8">
    <title>uncss TypeError demo</title>
    <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/normalize/4.1.1/normalize.min.css">
    <style>
        .content {
            margin: 20px;
        }

        .red {
            color: red;
        }

        .green {
            color: green;
        }
    </style>
</head>

<body>
    <div class="content">
        <span class="green">This text should be green and change to red after page load.</span>
    </div>
    <script src="jquery.slim.min.js"></script>
    <script>
        $(function() {
            $(".green").removeClass("green").addClass("red");
        });
    </script>
</body>
</html>

output css

/*** uncss> filename: https://cdnjs.cloudflare.com/ajax/libs/normalize/4.1.1/normalize.min.css ***/html{font-family:sans-serif;-ms-text-size-adjust:100%;-webkit-text-size-adjust:100%}body{margin:0}::-webkit-input-placeholder{color:inherit;opacity:.54}::-webkit-file-upload-button{-webkit-appearance:button;font:inherit}/*# sourceMappingURL=normalize.min.css.map */

.content isn't there at all.

@mikelambert
Copy link
Collaborator

Sigh. These are probably best discussed on another bug. I am not clear why you're trying to discuss a hodgepodge of random complaints in this bug, since AFAICT this bug has nothing to do with these two issues you just brought up?

Can you please file individual requests, one for each independent issue, to help keep this project clean and coherent?

I will be happy to respond to your complaints (to explain why they are impossible and/or detrimental to your own stated end goals) in any individual bugs you file, since it does not make sense to do this here, as that will only continue to spam the others in this thread who do not care about your personal bugs.

Thanks

@LuisAlejandro
Copy link

For everyone still having this error, this could be happening to you: #268 (comment)

@uncss uncss deleted a comment from cresolindiateam Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants