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

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

Open
mapio opened this issue Dec 4, 2015 · 12 comments
Open

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

mapio opened this issue Dec 4, 2015 · 12 comments

Comments

@mapio
Copy link

mapio commented Dec 4, 2015

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

var parse = require('esprima').parse;

a missing library.

@warpig9
Copy link

warpig9 commented Dec 10, 2015

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

@mapio
Copy link
Author

mapio commented Dec 10, 2015

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
Copy link

warpig9 commented Dec 11, 2015

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
Copy link

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

@mapio
Copy link
Author

mapio commented Dec 11, 2015

@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:

  • update the dist directory to contain working (built) files,
  • remove the dist directory and update the documents where it is said that it's enough to download files from such dir.

@socketwiz
Copy link

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
Copy link

magwas commented Dec 21, 2015

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
Copy link
Contributor

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

@mapio
Copy link
Author

mapio commented Jan 26, 2016

@stefaneg as you see in #541 (comment) @warpig9 does not consider this a bug :( so no fix will be ever provided…

@stefaneg
Copy link
Contributor

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
Copy link
Author

mapio commented Jan 28, 2016

@stefaneg 👍

@louisatome
Copy link

+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.

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

7 participants