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

apps.js (multi-build) should be more careful when joining scripts, affects steal loading #36

Closed
asavoy opened this issue Aug 10, 2011 · 5 comments

Comments

@asavoy
Copy link
Contributor

asavoy commented Aug 10, 2011

I'm running JavascriptMVC 3.1 without updating. I had issues with multi-build in my app, where in production mode only, my page's main controller was not be initialised (i.e. the callback in the "then()" clause), even though its dependencies were being loaded. Works in dev mode. I've described the details in a post on the forums: http://forum.javascriptmvc.com/#Topic/32525000000609003

In steal/build/apps/apps.js:181 there is a line:

var source = "steal('//" + paths.join("'\n,'//") + "');\nsteal.end();\n" + src.join(";steal.end();\n"),

which joins scripts into a single package file.

This doesn't work if one of the scripts has a single-line comment at the end of the file - the packaged script ends up looking like:

steal.plugins('jquery/controller').then(function($){
}); // end closure; steal.end();

The fix is to add a newline in there, so the apps.js:181 line becomes:

var source = "steal('//" + paths.join("'\n,'//") + "');\nsteal.end();\n" + src.join("\n;steal.end();\n"),
@lastzero
Copy link

A similar problem is the use of UTF-8 byte order marks in any file, as far as I remember from a past experience. The fix with the added new line char seems valid - but I'd like to wait for feedback from Brian or Justin.

@moschel
Copy link

moschel commented Aug 18, 2011

Yeah that fix looks good, but steal.end() isn't part of the latest steal anymore, so this might no longer be needed in 3.2.

Michael, what is the case where it broke for you? Is this the same fix for your issue? Does it still apply with the latest steal?

@lastzero
Copy link

Might fix it or not. Might also just be a problem when including css or less files (can't remember it anymore in detail). But I guess it's a good enough fix to simply disable byte-order marks in the editor - I don't know which editors add them, but there seem to be some.

@moschel
Copy link

moschel commented Aug 18, 2011

Ok. Well in that case I'm going to close this. If either of you find a breaking case still, let me know, or better yet, submit a pull with the breaking case.

@moschel moschel closed this as completed Aug 18, 2011
@asavoy
Copy link
Contributor Author

asavoy commented Aug 19, 2011

I would strongly suggest that there is some kind of warning, if a UTF BOM is found during build.

I have been bitten by the UTF BOM affecting build of production css files and I wasted several hours trying to work out why there was a difference between production and dev. I can't recall if it broke the first style in the file, or if it prevented the whole file from being included. Some editors will auto-inject a BOM when you type a non-ascii char, and other times, you may include a third-party css file without knowing that it's UTF-8 + BOM format.

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

3 participants