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

Join Javascript files using ';' instead of a newline #100

Closed
mvantellingen opened this issue Feb 7, 2012 · 10 comments
Closed

Join Javascript files using ';' instead of a newline #100

mvantellingen opened this issue Feb 7, 2012 · 10 comments

Comments

@mvantellingen
Copy link

Hi,

The bootstrap 2.0 js files minified with rjsmin throw an error: "Uncaught SyntaxError: Unexpected token !"

Problem is the !function() {} syntax

@miracle2k
Copy link
Owner

Have you reported this to the rJSmin maintainer?

@mvantellingen
Copy link
Author

Hey,

Sorry for the short description. I've also tried uglifyjs which also doesn't seem to work. Something weird is going on but i'll try to find out if it really is an issue in webassets or something else.

@sean-lynch
Copy link

Running into the same thing. It looks like it's an issue with Bootstrap 2.0's use of ! to define self-calling functions instead of ( ). YUI's compressor seems to handle it just fine so that's a short term fix.

I'm assuming you're bundling with other js files. If the JS file before bootstrap in the list ends in a semi-colon, that should also solve the problem.

@sean-lynch
Copy link

Scrap my second line, the semi-colon is not a general work around.

@sean-lynch
Copy link

Yep, confirmed as an issue with Bootstrap and jsmin. I've asked them to fix it:
twbs/bootstrap#266

@miracle2k
Copy link
Owner

I see, so the problem is not the ! per se, but the lack of a semicolon at the end of the files, which results in potentially invalid Javascript code, depending on how the next file begins. The reason then, I suspect, why YUI might work in some cases when rJSmin or UglifyJS do not, is that YUI is probably smart enough to recognize cases where the linebreak that webassets inserts is of actual importance as a separator, whereas these others remove the linebreak.

However, as is pointed out here:

twbs/bootstrap@68605bd#commitcomment-620727

there are cases where even a linebreak will not be enough.

I don't think bootstrap will fix this - the decision to remove the semicolons again was quite concious. They provide preminified versions instead. While I am not using bootstrap myself, I wouldn't find that satisfactory, since I like to use uncompressed third party libs in development.

Which leaves the question whether webassets shouldn't simply insert a semicolon between Javascript files. If I understand the Javascript Syntax rules correctly (a lone semicolon is a valid statement), then this shouldn't cause any problems, right?

@sean-lynch
Copy link

I'm with you in that I prefer the uncompressed third party libs.

Exploring the Bootstrap issue more, it looks like the issue isn't the semicolon between Javascript files, it's because bootstrap has several top level modules in the same file. Webassets would have to re-write the entire file in order to get it to pass. Seems like that might be out of scope?

miracle2k added a commit that referenced this issue Feb 11, 2012
@miracle2k
Copy link
Owner

I don't think so - there is nothing special about the way the bootstrap Makefile joines the file. They are simply using uglifyjs, which actually seems to be able to deal with this situation properly - rJsMin indeed is not.

Truth to be told, I don't expect this to be fixed in rJsMin. It might actually be not that easy even for a regular expression based parser, no idea.

I have just added a way to customize the separator. This is mainly for testing now, and may not stay, or not in this form:

bundle = Bundle(....)
bundle.joiner = ';'

This will join the source files using a semicolon, which I believe should help. The question is whether there are any side effects. I'll be using a semicolon for my JS bundles from now on for testing, and we'll see.

@sean-lynch
Copy link

Just a heads up, looks like Bootstrap is going to add semicolons back in the next version:
twbs/bootstrap#266 (comment)

@stewartadam
Copy link

stewartadam commented May 11, 2018

For those finding this today, bundle.joiner no longer exists and the functionality was merged into the concat filter method instead. You have to implement it like this:

class ConcatFilter(Filter):
    def concat(self, out, hunks, **kw):
        out.write(';'.join([h.data() for h, info in hunks]))

js = Bundle(
    'a.js',
    'b.js',
    filters=(ConcatFilter, 'jsmin'),
    output='app.min.js'
)

I ran into this as two Immediately Invokable Function Expression (see here) in different files without a semicolon to separate them cause a Uncaught TypeError: (intermediate value)(...) is not a function.

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

4 participants