Sitelet https://web.archive.org/web/20201109114738/https://github.com/socketio/socket.io/pull/2776
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

Serve slim client #2776

Open
wants to merge 5 commits into
base: master
from
Open

Conversation

@benjamin-albert
Copy link

@benjamin-albert benjamin-albert commented Dec 1, 2016

The kind of change this PR does introduce

A code change that improves performance.

Current behaviour

The slim client is not served.

New behaviour

The new slim client (and its source map) are served.

Other information (e.g. related issues)

This refactor also:

  1. Gzips the response.
  2. Streams the files as binary, instead of buffering strings on init and converting the strings back to binary on each request.
  3. Removes code duplication for source map serving.

I also added test for serving the source maps and slim client.

res.writeHead(200);
res.end(clientSourceMap);
var readStream = createReadStream(path.join(clientRoot, file));
if (~req.headers['accept-encoding'].indexOf('gzip')) {

This comment has been minimized.

@benjamin-albert

benjamin-albert Dec 1, 2016
Author

I'm not sure this if is necessary.

I don't think we support any browser that doesn't support gzip.

@benjamin-albert benjamin-albert changed the title Slim client Serve slim client Dec 1, 2016
@darrachequesne
Copy link
Member

@darrachequesne darrachequesne commented Dec 7, 2016

Hi, thanks for the PR!

A question though: shouldn't the gzip work rather be handled by some reverse proxy?

@benjamin-albert
Copy link
Author

@benjamin-albert benjamin-albert commented Dec 7, 2016 •

@darrachequesne Thank you for bringing up this question!

To be quire honest the main reason I opened this pull is so I can update examples with:

<script src='/scoket.io/socket.io.slim.js'></script>

However as far as I can tell, examples are the only reason Socket.IO should be dealing with the (pretty big) burden of serving static files.

When I use Socket.IO I:

  1. Use it from a node.js (or other clients like the java client) which doesn't need a web server to serve the client files.
  2. Include the client in my webpack/browserify bundle.
  3. Use CDNJS which already does the minifying and gzipping for me and completely removes the burden of serving the client from me.

In fact the chat example is the only time I ever used this feature.

However I understand that all of these options are a nightmare for documentation because:

  1. You will have to tell uses to look in package.json to find the server version.
  2. Tell the user to install/use the correct client version.

Bottom line

Setting up a reverse proxy can sometimes be more complicated (and might involve corporate politics) compare to one of the options I suggested above.

You never used this feature because you care about micro optimizations (removing the close to nothing burden of gzipping the client).

You use it because it already does everything for you, and gzipping should be one of those things especially with the growing number of users on mobile.

@benjamin-albert
Copy link
Author

@benjamin-albert benjamin-albert commented Dec 8, 2016

If gzipping is stopping the PR from being merged we can move that to a separate pull request so I can update the docs with the new slim client.

@darrachequesne
Copy link
Member

@darrachequesne darrachequesne commented Dec 8, 2016 •

@benjamin-albert actually, I would like to fix the issues related to minified/map files before merging your PR:

socketio/socket.io-client#1040
#2777

PR => socketio/socket.io-client#1042

@ericmandel
Copy link

@ericmandel ericmandel commented Jul 9, 2019

@darrachequesne I also need to load the slim version of socket.io.js using something like:

<script src='/sitelet?url=https%3A%2F%2Fweb.archive.org%2Fsocket.io%2Fsocket.io.slim.js'></script>

because (long-story short) loading socket.io.js (with its debug code) into a web page controlled by Electron.js can trigger a full 4 second delay during compilation ... if more than one instance of the Electron-based app is running simultaneously. Can you please advise about how to proceed? A new PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.