jspm / project

Roadmap and management repo for the jspm project
161 stars 8 forks source link

jspm.dev module is broken in Safari v13 due to encoding issue. Switching to another CDN instantly fixed it. #97

Closed varenc closed 2 years ago

varenc commented 3 years ago

I was trying out this cool project: https://github.com/hsablonniere/cistercian-numerals

So I made a page that was essentially just:

...
<script type="module" src="https://jspm.dev/cistercian-numerals"></script>
<cistercian-clock>
...

Which worked totally fine in Chrome. When I tried Safari v13.1.3 I got this mysterious error with no line number: SyntaxError: Invalid character '\u00a3'

I think it has something to do with the the encoding/headers marking encoding served by jspm.dev? Possible the module is at fault but not sure.

Switching my html file to use this CDN instead: <script type="module" src="https://cdn.skypack.dev/cistercian-numerals"></script> fixed it.

Hinting at the cause of the problem if I try to wget the module from jspm.dev like this: wget https://jspm.dev/npm:cistercian-numerals@1.1.0

Then the result is some useless seeming binary file.

$ less npm:cistercian-numerals@1.1.0
"npm:cistercian-numerals@1.1.0" may be a binary file.  See it anyway?
guybedford commented 3 years ago

Thanks for posting - I wasn't aware that Safari 13.1.3 didn't support Brotli - this is a known issue in that the server only supports Brotli compression and not Gzip compression currently as the idea was that all browsers that support ESM also support Brotli, I missed this Safari case it seems though!

I'm hoping to get a fix out for this in the next couple of months - it is a high priority to solve. Apologies for the issue, will keep this open until then.

varenc commented 3 years ago

Ah so it's brotli's fault! that makes complete sense why the wget version was a binary block since it doesn't support brotli.

But besides Safari's lack of brotli support, I think another issue is that the jspm.dev server is returning brotli compressed content even when the request doesn't include "content-accept: .., br"? Wget doesn't advertise it supports brotli that so it's odd that jspm.dev still returns brotli encoded data.

Confusing things more, when I go directly to "https://jspm.dev/npm:cistercian-numerals@1.1.0" in Safari v13 and look at the request, I see that Safari DOES include "content-accept: .., br" in its request, jspm.dev returns brotli encoded content, and Safari then display the content just fine. For some reason the issue is only showing up when the ESM is included in Safari. Perhaps it doesn't support brotli in that context? ¯_(ツ)_/¯

Cheers!

varenc commented 3 years ago

Actually it doesn't seem related to brotli at all. Safari v13 does support brotli even for ESMs. image

I think the problem is the Content-Type header. jspm.dev returns Content-Type: application/javascript but skypack returns Content-Type: application/javascript; charset=utf-8. My guess is Chrome just does a better job at guessing the correct charset. If I use Charles proxy to intercept the response from jspm.dev and add charset=utf-8 to its Content-Type response header, then things work fine in Safari! So for this I think the fix is just as simple as indicating that charset in the Content-Type.

Cheers!

guybedford commented 3 years ago

@varenc just realized you have the perfect username for this discussion :P

So if the issue is the charset, I'd suggest trying an HTML page with <meta charset="utf-8"> and seeing if that helps the issue?

I can certainly update the headers to avoid this problem in future.

varenc commented 3 years ago

I never considered that! it was fate! That should inspire me to actually understand this topic better..

I just tried adding a <meta charset="utf-8"> header on my page using the <cistercian-clock> and no luck. I think that only specifies the character encoding (is that the right word?) for that page and not for externally fetched resource.

My guess is this doesn't matter at all for most packages, but this one just contains some weird characters where the charset can't be guessed and where doing it correctly actually makes a difference. When I compare the results of jspm.dev with and without the charset=utf-8 header, the only differences are things these:

image

guybedford commented 3 years ago

@varenc I've fixed up this specific case to use a charset-utf8. Let me know if that fixes the issue here.

Backporting the fix to all packages will still take some time as it involves flushing the previous builds / running an update, I will leave this open to track that.

guybedford commented 3 years ago

(All new builds will get this charset though)

certainlyakey commented 1 year ago

Actually it doesn't seem related to brotli at all. Safari v13 does support brotli even for ESMs. image

I think the problem is the Content-Type header. jspm.dev returns Content-Type: application/javascript but skypack returns Content-Type: application/javascript; charset=utf-8. My guess is Chrome just does a better job at guessing the correct charset. If I use Charles proxy to intercept the response from jspm.dev and add charset=utf-8 to its Content-Type response header, then things work fine in Safari! So for this I think the fix is just as simple as indicating that charset in the Content-Type.

Cheers!

@varenc your comment allowed us to resolve a strange issue where Safari <16.4 would display garbled characters instead of Arabic when those would be contained in a JS file (as a part of a relatively common JS app). Even though the file would be utf-8-encoded, Safari would still convert it to some other encoding according to some mystery heuristic. This would also happen only on some servers, which was driving us crazy.

This was happily fixed in 16.4 (look for "Fixed module scripts"). For earlier versions we fixed it (following your advice) by setting Content-type header for all JS files to application/javascript; charset=utf-8 instead of the default application/javascript.

This was a major wall-banger, so huge thanks for sharing it 😁