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

http: support serving wasm by StaticFileHandler #6377

Merged

Conversation

makenowjust
Copy link
Contributor

This is import to serve wasm file. Browsers don't run wasm without correct 'Content-Type'.

@straight-shoota Can you add wasm mime type to #5765 when this is merged?

This is import to serve wasm file.
Browsers don't run wasm without correct 'Content-Type'.
@straight-shoota
Copy link
Member

I'm not sure about extending the default MIME type list with such lesser used types.

This is just adding a random type. There are hundreds of MIME types a webserver needs to know about. There are many much more important than .wasm (just think about image formats like PNG or GIF). If we start going this path, we should go even further and provide a more extensive MIME database from the start.

The trouble is, if we include a considerably sized database by default, people will expect it to be comprehensive. And I don't think the Crystal developers should be bothered with maintaining this. That's typically the system or application administrator's job. If we just provide a minimal default set, it is clear that this is not meant for anything useful and you need to use a proper MIME database. With #5765 this is going to be much easier and typically don't require any configuration whatsoever.

The existing defaults are really just a bare minimum to get at least "something" going, but can't honestly provide anything useful.

@makenowjust
Copy link
Contributor Author

@straight-shoota You thought the importance of wasm on Web is tooo little. It is the standard ranked with JavaScript in near future.

And other important point is, repeatedly, browser does not accept wasm file without correct Content-Type. This experience is so bad. What do you thikn if the image served by your server is not shown in browser?

@asterite
Copy link
Member

Come on, it's just one line, it doesn't hurt anyone :-)

@straight-shoota
Copy link
Member

@makenowjust I know the current state is not satisfactory. That's why I did #5765. That's actually the better solution to your problem. Let's wait for it to get merged, and there shouldn't be any issues with missing MIME types (or it's easy to fix with MIME.register(".wasm", "application/wasm")).

@RX14
Copy link
Contributor

RX14 commented Jul 13, 2018

@asterite it'll be 2000 lines if we allow crystal to maintain it's own complete mime registry. The fact is that wasm isn't that common a mime type. It's even too new to be in my /etc/mime.types! We should merge the mime registry, then people can super easilly register their own mime type in their own apps with one line of code.

@makenowjust
Copy link
Contributor Author

It is true #5765 is better of course 😄

I don't hope crystal to maintain the entire mime db. It is too large. I think so too. I hope only to add just one line. In other words, wasm mime type should be contained in the smallest fallback mime db of crystal. I feel it is valuable because I'm sure wasm becomes the important Web standard and it can prevent the worst experience: wasm is not ran due to no mime type. In fact Go's standard library contains wasm mime type.

To be honest I'm embarrassed to need to use the server written Go to develop wasm 😢

@asterite
Copy link
Member

Here's the thing, which happens a lot in this project: we have a simple fix that can improve a developer's life. Then we have a monolith that's a super good, flexible alternative, but it's being discussed, it's in review, etc. People in this community prefer to not add a single line now, which will be gone anyway later, and instead stay with a poor solution until the really good one comes. I'm seeing this a lot lately, unfortunately.

@RX14
Copy link
Contributor

RX14 commented Jul 14, 2018

My only concern is that this line of code wouldn't be removed by #5765. But since you can just do MIME.register(".wasm", "application/wasm") in your user code until wasm makes it into the standard mime.types, it shouldn't be too big a deal to not include this in #5765's fallback registry.

@RX14 RX14 merged commit 12e2958 into crystal-lang:master Jul 14, 2018
@RX14 RX14 added this to the Next milestone Jul 14, 2018
@makenowjust makenowjust deleted the feat/support-wasm-static-file-handler branch July 14, 2018 15:21
@Sija Sija mentioned this pull request Jul 14, 2018
@RX14 RX14 modified the milestones: Next, 0.26.0 Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants