lessc compile error "Cannot call method 'charAt' of undefined" #592

Closed
philipp-spiess opened this Issue Jan 23, 2012 · 43 comments

Projects

None yet
@philipp-spiess

Hi,

When trying to compile Twitter's bootstrap via lessc, i found out this very strange behavior.

When i'm compiling within bootstrap's directory, everything works fine:

$ cd bootstrap && lessc bootstrap.less

But when i go up one folder, it looks like

$ cd ../ && lessc bootstrap/bootstrap.less
TypeError: Cannot call method 'charAt' of undefined
    at getLocation (/usr/lib/node_modules/less/lib/less/parser.js:204:34)
    at new LessError (/usr/lib/node_modules/less/lib/less/parser.js:213:19)
    at Object.toCSS (/usr/lib/node_modules/less/lib/less/parser.js:379:31)
    at /usr/lib/node_modules/less/bin/lessc:103:28
    at /usr/lib/node_modules/less/lib/less/parser.js:428:40
    at /usr/lib/node_modules/less/lib/less/parser.js:94:48
    at /usr/lib/node_modules/less/lib/less/index.js:113:15
    at Object.parse (/usr/lib/node_modules/less/lib/less/parser.js:430:17)
    at /usr/lib/node_modules/less/lib/less/index.js:112:14
    at [object Object].<anonymous> (fs.js:107:5)

Why i need this? I've included bootstrap to my project via @import. This inclusion file is one dir above bootstrap.less. So when i'm compiling my inclusion file, it gives me the very same error.

Additional:

$ lessc -v
lessc 1.2.1 (LESS Compiler) [JavaScript]
$ uname -a
Linux gamboo.at 2.6.32-4-pve #1 SMP Mon May 9 12:59:57 CEST 2011 x86_64 GNU/Linux

I hope someone can reproduce this error.

Some additional information:

  • I'm running Debian
  • Inclusion via browser works great
  • Stable v1.4.0 of Twitter's bootstrap in the same procedure also works great

And another intressing part:

When cd-ing to bootstrap/ and building via lessc ../include.less everything works fine :D

@Anahkiasen

That error is the error I get everytime I have an error in one of the file I imported. It's actually kind of not handy, everytime there is a typo or an error in a file that is not the main file, you have no way to find out what it is unless you recompile each imported file one by one until you get the one that throws an error and tells you where it is.

@carlsverre

The issue is that getInput() looks up file contents by e.filename and e.filename seems to be set differently depending on where you are relative to the target file.

Here is a example node debugger session that shows the issue:

break in node_modules/less/lib/less/parser.js:195
193
194     function getInput(e, env) {
195         debugger;
196         if (e.filename && env.filename && (e.filename !== env.filename)) {
197             return parser.imports.contents[e.filename];
debug> repl
Press Ctrl + C to leave debug repl
> e.filename
'/Users/carl/dev/web/assets/css/test/colors.less'
> parser.imports.contents
{ 'test/colors.less': 'body {\n    color: #330033;\n};\n' }

To the developers:
I would suggest normalizing the keys you use to store data in parser.imports.contents. Check out require('path').relative. Given a base path and a full path, it will normalize it to essentially the difference between the two paths which gets you the desired cache_key.

require('path').relative('/tmp/something/assets/css', '/tmp/something/assets/css/test/colors.less')
    => "test/colors.less"
@carlsverre

Heh, shoulda checked the recent commits first. Looks like your working on this issue in 557177c

@purcell
purcell commented Feb 22, 2012

I get the same problems when using browser inclusion: I have an "application.less" under "/stylesheets" which then includes bootstrap's main Less file from a subdirectory:

@import "bootstrap/bootstrap.less";

then my page source looks like this:

<link href="/stylesheets/application.less" rel="stylesheet/less">
<script src="/javascripts/less-1.2.2.min.js?1329833989" type="text/javascript"></script>

resulting in the error:

Cannot call method 'charAt' of undefined

in sprites.less 
at Object. (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:9:9419)
at Object. (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:12000)
at w (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:6703)
at Object.entity (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:14070)
at w (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:6703)
at Object.expression (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:19055)
at w (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:6703)
at Object.value (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:17585)
at w (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:6703)
at Object.rule (http://localhost:3000/javascripts/less-1.2.2.min.js?1329833989:8:16106)

Interestingly, the "sprites.less" reference in the error is hyperlinked to "/sprites.less", which suggests that Less is confused about the paths.

Compilation using "lessc" works fine, so I'm seeing almost the opposite behavior to that observed by @philipp-spiess above.

-Steve

@paulogaspar7
Contributor

I identified the commit where this problem started, as you can see here:

@paulogaspar7
Contributor

[UPDATED]

I did try with compiling my stuff with 1.2.2 but it still has the same problem.

I finally diagnosed the problem and got to a solution I find acceptable. It is a problem related with scopes and timing.

The callback you pass when you call less.Parser.importer() at the top of "parser.js" is at the top importing files Parser scope. However, I think this callback only gets called and registers the content data (the actual less code text) ater an import is parsed. This might be to late for some parsing errors.

Anyway, when there is a parsing error - because there is actually an error on the Less source code which trigger this problem - the code handling the error does not have access to part of the data it counts on to perform its task.

As parte of the error handling, Less tries to display the offending code. It uses the getInput() function (at parse.js) to get that offending code. When it gets an undefined instead of a string of Less code and tries to cut a bit of it to display it, it cause an exception to be thrown. That is the origin of the weird error message reported at issue #592.

This is also why the error is intermittent. The problem at the error handling code is simply hiding the error message for the real problem. Eventually, the programmer fixes the original problem at his Less code and everything works again.

Therefore I tried to load the cache as soon as the less stylesheet code is loaded. Then, of course, it is necessary to pass the data to the top importing parser cache, which requires some reference passing.

I found a solution simple enough for me to feel confortable. Not beautiful, but I don't feel confortable making a big change on your code. Had to keep it as simple as possible.

I tested everything by:

  • Running the tests via "make test";
  • Compiling via "lessc" the Less stylesheets for the current (straight rom the master) Twitter Bootstrap and from the app I am working on;
  • Running Bootstrap with dynamic loading too.

Everything worked fine except for an unrelated issue, present also on 1.2.2 - resolving variables only on dynamic loading on url()s, like this one:

I also removed the use of basename() since it was vulnerable to name clashes - e.g.:

  • when multiple libraries have a "variables.less" file;
  • and there is a single "main.less" stylesheet importing from all libraries.

In my use cases I detected no problems about that either and I was careful to ensure all bits and pieces received as filename the name actually used by the loading logic (for both browser and node).

I will now commit and submit a pull request.

Cheers

@paulogaspar7
Contributor
@rainboxx

I'm also running into this issue. Hope that fix will get applied and a new release will be issued soon.

@nvk
nvk commented Feb 28, 2012

Same here, any news?

@davps
davps commented Mar 2, 2012

I'm watching for a fix applied to the new release too.

Thank you very much!

@paulogaspar7
Contributor

Any hope on fixing this? The above mentioned patch passed tests and worked on my own tests both from command-line and at the browser.

@danawoodman

I'm also hitting this problem. Any file that matches the same name as a boostrap file causes the issue.

@paulogaspar7
Contributor

According to my tests, master (and the new version) still have this bug.

@paulogaspar7
Contributor

New pull request with fix for this error. Still same changes. Still passes of tests I have available.

@davidhund

I am also running into this bug (with less 1.3.0 and Bootstrap 2.0.2 WIP).

My file imports bootstrap from a subfolder. There are no files with identical names.
The issue with the sprites is (I think) a different one, the TB people have been/are working on a fix: twbs/bootstrap#2029

Is there a workaround for this issue related to importing TB form a subfolder?

@rkrzr
rkrzr commented Mar 29, 2012

This bug is still happening for me with the latest code.
It runs fine with less@1.2.0 for me.

@lvh
lvh commented Mar 30, 2012

Same issue here. Is there a workaround for @davidhund 's situation that anyone has found?

@rainboxx

I don't experience this issue anymore since the update to the last version of Bootstrap. (They mentioned a fix for this, I remember...) Maybe the issue was on Bootstrap's side.

@lvh
lvh commented Mar 30, 2012

I am still having the issue. I have a submodule in ext/bootstrap that's up to date with twitter/bootstrap.

Here is my file structure:

lvh@thoreau ~/Code/bacon/igloo ±master⚡ » tree -L 2
.
|-- ...
|-- docs
|   `-- ...
|-- ext
|   `-- bootstrap
|      `-- ...
`-- igloo
    |-- cs
    |    `-- ...
    |-- ...
    `-- style
        `-- igloo.less

Currently, igloo.less just contains:

@import "./ext/bootstrap/less/responsive.less";

Compiling it from the base gives the known error:

lvh@thoreau ~/Code/bacon/igloo ±master⚡ » lessc igloo/style/igloo.less
TypeError: Cannot call method 'charAt' of undefined
@rainboxx

Uhm, maybe I have to say that this doesn't happen with the JS version of LESS. Haven't tried to compile it through the CLI yet... Thanks @lvh for remembering it to me :-).

@paulogaspar7
Contributor

Last time I checked it was happening with both versions. Maybe I should create a simple repro case, since this issue is already shamefully old, I got no feedback from the pull request I submitted and we still have this kind of doubt.

@cowboyd
Contributor
cowboyd commented Apr 4, 2012

The underlying problem here is structural. For a bit of .less source there are two concepts: the actual physical resource from which it was loaded, which is the filename, and in the case of imports, the logical path: the name you use do an @import with.

Nodes in the syntax tree are only aware of the former, not the latter, so when generating errors, there is simply no clean way to reconcile a node to the logical path where its contents are cached. Rectify this structural deficiency and fixing this error will shake out nicely.

@paulogaspar7 This doesn't have anything to do with timing since I see it also in less.rb which runs in a fully synchronous fashion.

@exortech
exortech commented Apr 4, 2012

here's a naive fix for this issue: https://gist.github.com/2306083

it will fail when importing different files with the same name (so something more robust is required). but if you're looking for a quick fix, this will at least give you an appropriate error message.

@cowboyd
Contributor
cowboyd commented Apr 5, 2012

@exortech Owen?

@exortech
exortech commented Apr 5, 2012

@cowboyd hey charles - long time, no see :)

@cowboyd
Contributor
cowboyd commented Apr 5, 2012

You old JavaScript hacker, you! @purcell and I were just commenting how wonderful it is to run into old friends on a github issue. Thanks for the workaround. I may even consider including it as a patch to less.rb until this issue is resolved.

Who knows, maybe we can come up with a longer term fix together, for old times sake :)

@paulogaspar7
Contributor

@cowboyd I never said it has anything to do with timing, did I?

@paulogaspar7
Contributor

@cowboyd You diagnostic is accurate.

If you look at the pull request I submitted, you will see that structural deficiency fixed. It could be nicer, but it sure works.
(Or worked, just a couple of weeks ago.)

@lvh
lvh commented Apr 14, 2012

+1, this works for me; what's the blocker?

@pogo19
pogo19 commented Apr 19, 2012

This simple fix doesn't help when compiling (using rhino, node, or browser)
https://github.com/addyosmani/jquery-ui-bootstrap/blob/Bootstrap2-Dev/css/less/style.less

@pogo19
pogo19 commented Apr 20, 2012

Pull request #703 from @paulogaspar7 fixes the problem for me. Thanks!

@jappievw
jappievw commented May 3, 2012

Good question @ivh. Hope to see it included soon!

@visualmotive

This has got to be the single most annoying problem with Less. I encounter it daily when debugging Less files.

@paulogaspar7
Contributor

@visualmotive Well, you can try applying my patch yourself, since the project committers are not picking it. =:oP

@scjody
scjody commented May 29, 2012

Downgrading to less 1.2.0 also works for me as a workaround if you can't run a non-released version of less.

@jasonmcleod

@exortech big thanks for posting (https://gist.github.com/2306083). I'm moving along again :]

@tsusanka

The fix doesn't work for me and the problem still persists.
lessc 1.3.0
ubuntu 12.04

@cboettig
cboettig commented Aug 4, 2012

+1 issue still exists for me on lessc 1.2.1, ubuntu 12.04

@cboettig
cboettig commented Aug 4, 2012

@exortech big thanks for posting (https://gist.github.com/2306083). I'm moving along again :]

@lukeapage
Member

Should be fixed now.

If it isn't a simplified test case for the new case would be good.

@lukeapage lukeapage closed this Aug 11, 2012
@exortech

any word on when this will be released?

@lukeapage
Member

Haven't discussed. Need to find out from @cloudhead.

@slavafomin

Still have this issue with lessc 1.3.0. I'm switching to 1.2.0 for now.

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