search

plurals / pluralize

Pluralize or singularize any word based on a count
MIT License
2.15k stars 184 forks source link

Minification broken for version 1.1.4 #24

Closed oaosman84 closed 9 years ago

oaosman84 commented 9 years ago

I'm pointing my bower.json to 1.1.2, and it just picked up version 1.1.4. During minification, it produced bad javascript, resulting in this error in the console:

Uncaught TypeError: (intermediate value)(...) is not a function

Can post more details here once I put out flames :) but my guess is missing semi-colon

blakeembrey commented 9 years ago

@oaosman84 What are you using for minification? It's likely the lack of a semicolon at the beginning is causing it issues if it's not a proper JavaScript parser.

Edit: If that is the issue, I can look at adding a semicolon for you - but hopefully it's a simple issue with the minifier :smile:

MikaAK commented 9 years ago

I'm having this issue as well, putting a semicolon at the end of the closure solves this.

blakeembrey commented 9 years ago

@MikaAK Thanks for the update. Would you mind testing a fix if I push one? It'll just be adding a semicolon to the beginning of the file, but I'm not sure what minifier you guys are using.

MikaAK commented 9 years ago

UglifyJS here I'll test the fix

blakeembrey commented 9 years ago

@MikaAK Thanks. Try from the semicolon branch (npm install blakeembrey/pluralize#semicolon). Are you perhaps concatenating the scripts together then?

MikaAK commented 9 years ago

@blakeembrey this is actually a ASI issue, you need to have a semicolon at the end of a closure or it shall break. I submitted a pr #26

blakeembrey commented 9 years ago

I know the issue, which is why I added a semicolon (just at the beginning, since I didn't know what it was being concatenated with). It's an issue with however you are concatenating it, not with the code though. Adding a semicolon is really a hack and this code will still break now if another library does not have an ending semicolon.

blakeembrey commented 9 years ago

The build is failing with it at the end too. I won't be merging a broken build. I'd accept putting it at the beginning, but you should fix the concat script you use too.

MikaAK commented 9 years ago

The concat script i use is gulp. It works for me when i put it after and doesn't work if i put it before.

blakeembrey commented 9 years ago

It breaks the formatting rules. I would add semicolons to your concat script (https://github.com/wearefractal/gulp-concat/blob/master/index.js#L20 looks like you could specify a semicolon) or add a semicolon before whatever you're concating onto the end of this. Future releases of pluralize will fix this by compiling the script for Bower users, but I expect it's a few weeks away. Or are you using npm? Using a JavaScript compiler for the browser will also fix this issue (E.g. browserify, webpack, etc.).

leobudima commented 9 years ago

I can verify that it breaks minification when using UglifyJS as well. Adding a semicolon at the end fixes it.

blakeembrey commented 9 years ago

I released v1.1.5, back to using semicolons. Future releases will always have semicolons.

Out of interest - how is everyone here installing it? Do future releases need to maintain a single file at the root (E.g. pluralize.js) for Bower concatenation? Using a build system should have mitigated any issues here.

leobudima commented 9 years ago

@blakeembrey - thanks a lot!

In my case, I'm installing through bower and using grunt with uglify to minify together with the rest of used libraries.

It's a dashboard / management type of app for a logistics company, written with AngularJS, and your library helps a lot throughout the UI to properly display labels and text for various models.

Thanks again for your help!