Skip to content
This repository has been archived by the owner on Oct 9, 2020. It is now read-only.

Ascii uglify flag #363

Closed
lookfirst opened this issue Oct 21, 2015 · 27 comments
Closed

Ascii uglify flag #363

lookfirst opened this issue Oct 21, 2015 · 27 comments

Comments

@lookfirst
Copy link
Contributor

This is angular.js source that is being minified with the builder and it is getting corrupted by the builder.

http://take.ms/y8MGr

latest builder/jspm/systemjs

@lookfirst
Copy link
Contributor Author

68cef2e

???

@lookfirst
Copy link
Contributor Author

@guybedford seriously, please write some tests around this stuff.

@lookfirst
Copy link
Contributor Author

reverting to "jspm": "0.16.12" fixes the issue

@guybedford
Copy link
Member

@lookfirst this is actually server-specific stuff. Try adding a <meta charset="utf-8"> tag to the main page and see if that helps?

@lookfirst
Copy link
Contributor Author

This has nothing to do with the server. The JS file that is served up is corrupted. I already have that header in my page. As I said, reverting to the previous version of jspm fixed this issue.

@lookfirst
Copy link
Contributor Author

Also, look at what I'm showing you... it is two JS files. One is the angularjs source code viewed on Github directly and the other is in developer tools. How can a meta header fix that?!?

@enlait
Copy link

enlait commented Oct 22, 2015

@lookfirst @guybedford There is nothing wrong with the builder output. This is an obvious encoding issue: utf8-encoded file is opened as win 1252. You can confirm it with any text editor at hand.

I vote for making ascii-only mode configurable through builder.

@lookfirst
Copy link
Contributor Author

@enlait I think you're pretty off there with your analysis... =(

@guybedford
Copy link
Member

@lookfirst here's my test:

  jspm install angular
  jspm bundle angular -mi

Loading that in a test page works completely fine for me. It's definitely an encoding issue, but I think the problem you are having is at the network not the filesystem level?

@enlait
Copy link

enlait commented Oct 27, 2015

@guybedford
can't blame @lookfirst, encodings are quite a quagmire and escaping unicode helps with compatibility. There are pros and cons for either way, in case of sparsely used special characters (which I understand is the case with angular) escaping is justified.

BTW isn't content-type-specified encoding more important then meta?

@lookfirst
Copy link
Contributor Author

bundler.buildStatic() with minification on?

@guybedford
Copy link
Member

@lookfirst if you open the file on the filesystem are the characters encoded correctly there? I added a simple test in 490fe61 and it's definitely working. I'd advise checking your server is setup to load and serve utf-8 correctly?

@lookfirst
Copy link
Contributor Author

@guybedford This is a bug. My 'server' is fine. I'm using Google AppEngine. Like I said before, this works fine in the previous version of jspm, it only breaks after you made that utf change.

@lookfirst
Copy link
Contributor Author

Your test is not accurate, you need to use the same characters as what I showed you in the angularjs source code. I also want to see this as part of the output of bundler.buildStatic() and with minification turned on.

@guybedford
Copy link
Member

Sure, see 18a2913, this is a test with both sfx and minification of the exact characters.

@lookfirst I appreciate your reports, but I hope you can work on your tone in these issue queues.

@lookfirst
Copy link
Contributor Author

I'm starting to debug this further. It only happens when I turn minification on.

@lookfirst
Copy link
Contributor Author

builder installs uglify-js 2.4.24

@lookfirst
Copy link
Contributor Author

echo "if (isInfinity) formatedText = '\u221e';" > test.js
uglifyjs test.js
if(isInfinity)formatedText="∞";
uglifyjs test.js > out.js
more out.js
if(isInfinity)formatedText="∞";

So basically, uglify is encoding things.

@lookfirst
Copy link
Contributor Author

uglifyjs -b ascii_only=false test.js
if (isInfinity) formatedText = "∞";
uglifyjs -b ascii_only=true test.js
if (isInfinity) formatedText = "\u221e";

@lookfirst
Copy link
Contributor Author

Let's not have uglify mess with the source code. =)

@lookfirst
Copy link
Contributor Author

Ok... so as a test, I made sure to send 'Content-Type: application/javascript;charset=utf-8' with the response. Never mind that I shouldn't have to do that.

What this does is make the browser handle the javascript ok... or at least it does for Chrome and Safari, Firefox on the other hand totally screws this up.

On the left is chrome, on the right is firefox (loading the same exact file): http://take.ms/pkbFi

I really recommend telling uglifyjs to not rewrite the unicode. Keep it as ascii.

@guybedford
Copy link
Member

I'd be happy to add a flag for this option certainly.

@guybedford guybedford changed the title minification is corrupting the file Ascii uglify flag Oct 27, 2015
@lookfirst
Copy link
Contributor Author

@guybedford Sweet, I can't upgrade to latest jspm until this happens. =(

Seems weird that your tests are passing. The input clearly isn't the same as the output here.

I'm also really surprised that I'm the only one having this issue. It really turns up if you use angular with $format('currency'). Maybe someone will search and find this issue.

@guybedford
Copy link
Member

Added in b1c09a5.

@lookfirst
Copy link
Contributor Author

👍 thanks @guybedford

@guybedford
Copy link
Member

Released in 0.14.10.

@lookfirst
Copy link
Contributor Author

👍 works

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants