-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Provide a slim build without JSON3 and debug #1030
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The slim client is now 16KB minified and gzipped verses the 22KB of the old IE support client (more than 27% smaller).
Really nice work! but please fix the issues I mentioned in the review before merging.
loader: 'imports?define=>false' | ||
}, { | ||
test: /\.js$/, | ||
loader: "strip-loader?strip[]=debug" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot to:
$ npm install strip-loader --save-dev
But after you install this everything works grate!
I'm impressed that not only you replace the debug module with a noop
but you actually strip calls to debug!
Another issue is ESLint complains when running gulp test
:
support/webpack.config.slim.js
30:15 error Strings must use singlequote quotes
This is causing the Travis build to completely fail.
@@ -0,0 +1,2 @@ | |||
|
|||
module.exports = function() { return function () {} }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESLint complains when running gulp test
:
support/noop.js
2:26 error Missing space before function parentheses space-before-function-paren
2:52 error Missing semicolon semi
This is causing the Travis build to completely fail.
How does 'aller un peu vite en besogne' translate? 😄 @benjamin-albert thanks a lot for the review, it should be better now. The output of webpack-bundle-size-analyzer now gives:
|
How do one use a slim version through the NPM package? |
I'm not sure this is the right place for how to questions like this. However I will provide instructions here since it might be useful to anyone else who finds this PR.
npm install socket.io-client --save-dev
ES6 import: import io from 'socket.io-client/dist/socket.io.slim.dev'; CommonJS require: const io = require('socket.io-client/dist/socket.io.slim.dev'); HTML script tag:
Exapress example: app.use('/socket.io-client', express.static('./node_modules/socket.io-client/dist'));
Minified production version: <script src="/socket.io-client/socket.io.slim.js"></script> Development version with lots of comments: <script src="/socket.io-client/socket.io.slim.dev.js"></script> @alexsorokoletov Please open a separate issue if you need help installing the slim client using TypeScript or any other JavaScript dialect/module system. |
@alexsorokoletov I don't think you should include the dist/ build directly, because this file is already minified/optimized for production. For example, if you have a dependency in common with the Socket.IO client, webpack - or another bundler - will have to include it twice in the final bundle. A better solution would be to use the webpack-remove-debug plugin directly in your webpack configuration. |
@benjamin-albert Thank you for the response, I was not sure also if it's the right place to ask. @darrachequesne very good point, that might not be an optimal solution. |
@alexsorokoletov actually, the |
Following the discussion here #1019: