-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Page Speed score #440
Comments
I'm not sure how to use a minified/compressed socket.io.js, because we're using the one that socket.io itself serves up. Maybe theres a setting to enable compression, but I'm not sure about minification. We could use a bower install and minify it ourselves, but that seems counter to the way socket io wants you to serve the client js. I definitely need to automate the rebuild and deployment of the fullstack demo. I could probably add it as part of the grunt task for when a new version is released to build and publish the demo again. |
@JaKXz Agree. I just copied ones from Page Speed, but it will be great if we completed this list with others as well, indeed :) |
Would it be overkill to have a few different combinations of setups (e.g. with/without mongo, with/without socketio) in different branches in the demo repo? They would of course be deployed to different heroku and openshift addresses. EDIT: the recent fix for |
@DaftMonk, @chester1000 take a look at this branch: feature/baked-in-socket.io I serve the socket.io.js client file with the default socket.io server when in development env, but when built we minify it into vendor.js and simply tell the socket.io server to not serve the client. I think i was able to keep it pretty simple and fairly lite in changes. Let me know if you think it would be suitable for a PR. @chester1000, not sure if you had started working on this or not. Just hoped i might be able to help a lil (: |
Also, I thought the JS was already in the |
I've added a grunt task to automate building and deployment of the demo. Not sure if we need different setups, seems a bit like overkill to me, but it wouldn't be hard to add now. @kingcody Nice solution. That would be welcome in a PR. |
@DaftMonk to |
@kingcody I think this is a small enough feature that it could go into master, as part of a patch. |
https://developers.google.com/speed/pagespeed/insights/?url=https%3A%2F%2Ffullstack-demo.herokuapp.com%2F&
Currently we score:
I bet we can do better! ;)
There are some things that can be fixed quite easily:
socket.io.js
,<head>
to<body>
(possibly more complicated),I wouldn't bother with above-the-fold stuff as that's the first thing anyone using this generator will change (unless there's a grunt plugin that can handle that somehow).
Also is https://fullstack-demo.herokuapp.com kept up-to-date with releases? @DaftMonk since you're the maintainer, how about adding
buildcontrol:heroku
task to generator's Gruntfile?The text was updated successfully, but these errors were encountered: