alex-seville / blanket

blanket.js is a simple code coverage library for javascript. Designed to be easy to install and use, for both browser and nodejs.
http://blanketjs.org
Other
1.4k stars 177 forks source link

Cannot read property 'parse' of undefined (at line 29 of blanket.js) #541

Open mapio opened 8 years ago

mapio commented 8 years ago

Serving the test dir and opening /backbone-koans/ in the browser result in the console reporting the error that is the subject of this issue. The culprit seems to be

https://github.com/alex-seville/blanket/blob/11f1a927477af163177a96df6590a397c1f4a0f5/dist/qunit/blanket.js#L29

a missing library.

warpig9 commented 8 years ago

I think you should built the project first. Follow the steps in Roll your own section.

mapio commented 8 years ago

I don't think you are right.

First of all, that file is a dist directory https://github.com/alex-seville/blanket/tree/HEAD/dist/qunit that suggest it already has been built.

Second, the same error (albeit at a different line number) happens if using the minified file in the same directory – exactly what one is suggested to do by the instruction in http://blanketjs.org/.

Finally, the error does not happen using https://raw.githubusercontent.com/alex-seville/blanket/89266afe70ea733592f5d51f213657d98e19fc0a/dist/qunit/blanket.js (without building).

So it seems that, probably by mistake, the wrong (unbuilt) file has been committed to the repo (in the dist directory).

warpig9 commented 8 years ago

Yes, the file blanket.js(65kb) in /dist/qunit is unbuilt. It became 200kb after I built it and that's why I suggest you should built it by yourself. BTW, from the source code I saw that blanket needs a 3rd-party parser(Esprima or other parsers), and the building work will import the parser in the target file, that's why blanket can do all the job in the browser without any nodejs dependency during runtime.

yangzhaox commented 8 years ago

I met the same issue. The files is /dist/qunit should be updated.

mapio commented 8 years ago

@warpig9 I can absolutely agree that committing a file that is built from other files in the repo is quite a bad idea (given its size, but not only); it is a common best practice not to do so, reported even on some GitHub guide http://kbroman.org/github_tutorial/pages/routine.html "[d]on’t include files that are derived from other files in the repository".

You should also consider that:

So I think you either to:

socketwiz commented 8 years ago

I just ran into this as well. Installing through npm has a similar problem. I had to clone the repo and rebuild the dist directory as well in order to get past it.

magwas commented 8 years ago

Same problem here. You should either keep your dist up-to date and built, or remove/rename it.

You know, we users are lazy. So lazy you should treat me as dumb. Because we have our own host of dependencies, and want to focus on our own project. So please provide ready-to-use package (in our case that one blanket.min.js is okay). I revert to https://raw.githubusercontent.com/alex-seville/blanket/89266afe70ea733592f5d51f213657d98e19fc0a/dist/qunit/blanket.js for the time being.

stefaneg commented 8 years ago

Backed out of 1.2.0 due to this issue. Any fix on the horizon?

mapio commented 8 years ago

@stefaneg as you see in https://github.com/alex-seville/blanket/issues/541#issuecomment-163816843 @warpig9 does not consider this a bug :( so no fix will be ever provided…

stefaneg commented 8 years ago

Either the documentation has to change or the code distribution. While they are not in sync, this is definitely a bug. I would suggest that the right thing to do is to publish a built blanket.js in a npm package, and update the docs accordingly.

mapio commented 8 years ago

@stefaneg :+1:

louisatome commented 8 years ago

+1 for @mapio arguments. That is really disturbing when you follow the docs and it doesn't work as expected. The docs should mention that or the file should be updated.