Add semicolon at the end of the zepto.js build #514

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet

Zepto's lack of semicolons was causing me trouble when concating with other files (namely underscore). I was ending up with something like this:

})(Zepto)
// End of zepto
// Start of underscore.js
(function () {

Which obviously causes issues. Putting in a final semicolon at the end lets zepto play nice with other scripts.

Collaborator

mislav commented May 13, 2012

This is a common issue when concatenating semicolon-less JS scripts. It's going to affect any JS file, not just Zepto. Therefore the solution is not to try to change all the libraries you use to at least include a semicolon at the end. The solution is to tell your concatenation program to put a semicolon in-between every file.

Our philosophy: libraries shouldn't change to appease tools.

@mislav mislav closed this May 13, 2012

Is this issue documented anywhere? I've had problems with concatenating libraries that break each other because of the lack of a semicolon. It seems like a problem that would be un-obvious to most people. Short of a code change such as this Pull Request, a note in the docs about concatenated closure invokation might be helpful to people.

Collaborator

mislav commented May 13, 2012

@jeremyckahn Yes, I can understand how encountering this error would be non-obvious, suprising and time-consuming. This is all the fault of tools which process JavaScript files, but change the code in a way that it no longer runs.

A notice in the docs might be helpful, although I'm not sure which our official site or the README is a better place to put it.

Yeah... I couldn't figure out where it should go. I'd be happy to add a note myself, perhaps this might be a good reason to start a Github wiki?

Normally I would agree with you. The reason I would make an exception is that Underscore/Backbone are probably the libraries most frequently paired with zepto (considering Backbone has first-class support for it). Zepto has made a choice to go against the grain with regards to coding style, which when isolated makes no difference, but can lead to confusion. It just seems like making integration with other libraries as friction free as possible would make sense so that know one has to know or care about Zepto's internal style and the potential issues that may arise when combining it with other projects that don't follow the same style.

+1 to @natefaubion. This seems like something that would benefit users and not negatively impact the project.

Edit: To clarify, I'm referring to the addition of a semicolon.

Contributor

elsigh commented May 22, 2012

The best part about this issue is that zepto-0.8 had a semicolon at the end, and that it's been intentionally removed.
Not including a semicolon at the end of the file thrusts potential pain on your lib's users. Following any convention you like in your lib is great, but not playing nice in what is a very typical JS production step, concat + min without a practical reason is slightly uncool.
I do love Zepto and the work ya put into it fwiw!

Contributor

paulmillr commented Jun 4, 2012

this is shitty, every popular lib respects asi. meh.

While I don't agree with not merging this change, perhaps we can find a peaceable resolution by building in semicolon-insertion into Uglifyjs? For all I know, the functionality is already there. If not, a semicolon-insertion patch in that project would have a greater impact than a similar one here.

Personally I don't have the time to dive in to Uglifyjs, but perhaps somebody else has the time or interest to make this a non-issue.

Collaborator

mislav commented Jun 5, 2012

I don't understand what needs to be changed about Uglifyjs. The zepto.min.js file produced by our build system and uglify has all the semicolons, including the trailing one.

I didn't realize that (I don't usually use uglifyjs). I guess it's a moot point.

domenic commented Jun 5, 2012

The problem is people who concatenate using cat. That's unreasonable; you need to use an ASI-aware tool to concatenate JS source files.

I agree with @domenic, but at this point we are beating a dead horse.

Just adding my 5 minutes of wasted time to the tracker... http://jelzo.com/stuff/zepto-broke-already.png Hope you fix this soon :).

What's ASI-aware mean?

cdata commented Nov 26, 2012

+1 to @natefaubion @elsigh and the rest. This issue is about making practical choices that enable JavaScript developers to do their job without having to worry about the philosophy of individual framework vendors. jQuery became and continues to be useful because it abstracts the developer from the tedium inherent in the underlying environment. A lot has been said in favor of and against relying on ASI, but it is unbecoming for a library written in the image of jQuery to get mixed up in that debate. Instead, it should just work.

chetzof commented Mar 4, 2013

"Our philosophy: libraries shouldn't change to appease tools."
Libraries should change to follow best practices, not to please their's author ego (like Twitter Bootstrap).

So instead of doing my job, i have to find a way to disable syntax check in my IDE (zepto is highligthed as one big error, lol) and prepare for potential problems during minification. For already crippled language like js with dozens of quirks and unexpected behaviours, not using ; brings extra problems.

Collaborator

mislav commented Mar 5, 2013

instead of doing my job, i have to find a way to disable syntax check in my IDE (zepto is highligthed as one big error

lol at your IDE indeed

[...] and prepare for potential problems during minification

Those pesky potential problems! They're almost as bad as actual problems.

Contributor

shalecraig commented Mar 5, 2013

Insulting intelligence to prove your point can be offensive.

Collaborator

mislav commented Mar 5, 2013

Sprockets, the assets preprocessor and concatenizer engine that ships with Rails inserts a semicolon by default when joining files. That's what I'm using, so you must understand why I don't care so much about fixing your broken tools. Add a semicolon if needed. It's not rocket science

HaNdTriX commented Jan 6, 2014

+1 to @natefaubion

Abstracting best practices from concatenation tools that try to fix potential bugs by adding a semicolon between each file seems to me like the wrong approach.

Anyway thanks for this almost perfect lib 😄

niels-garve added a commit to GetAmpersand/WebApp that referenced this pull request Aug 25, 2014

- added i18next translation
- fixed script concatenation (see madrobby/zepto#514)
- implemented (very) basic backbone rendering of Ampersand channels
- gulp: removed stream queue, now filtering for coffee script files
#1

@mislav mislav referenced this pull request in defunkt/jquery-pjax Sep 11, 2014

Closed

Improve minification support #434

@madrobby madrobby locked and limited conversation to collaborators Oct 6, 2014

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