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

Compile assets with grunt #730

Merged
merged 1 commit into from
Sep 16, 2013
Merged

Conversation

jgable
Copy link
Contributor

@jgable jgable commented Sep 14, 2013

  • Made a helper called ghostScriptTags that will spit out the relevant
    script tags with version parameter; 4 unminified files in development,
    1 minified file in production.
  • Added grunt concat and uglify tasks to build files into core/built
  • Fixed some unit tests by making them native date objects

Some off the cuff metrics on this that I could measure with the chrome dev tools

  • Number of Requests on /ghost:
    • 55 => 5 (10 in development)
  • Load time on /ghost:
    • ~260ms => ~140ms (~165ms in development)

The request size has also gone down but I'm not sure how to measure that fairly because of all the 304's and what-not after the first time you load the page.

I tested out the build task by running grunt build and then going to .build/build/ and running node index.js and NODE_ENV=production node index.js. Tell me what you think and let me know if you want anything changed in it.

@ErisDS
Copy link
Member

ErisDS commented Sep 14, 2013

Not sure what you think, but I much prefer this approach?

@jgable
Copy link
Contributor Author

jgable commented Sep 14, 2013

Updated with version from package, set a maxAge of one year and added the compress middle ware for gzipping. Also, moved the helper to the core helpers module and rebased on master.

@jgable
Copy link
Contributor Author

jgable commented Sep 14, 2013

Hold off on this, something is broken and the server is not loading. Trying to figure it out now.

EDIT: Found it, was a typo on server.use.

@jgable
Copy link
Contributor Author

jgable commented Sep 14, 2013

Updated again with @javorszky validation script included and had to add a semi-colon to the end of shortcuts.js because it was breaking when concatenated.

@ErisDS
Copy link
Member

ErisDS commented Sep 14, 2013

Does the helper work from the core helpers file? It's used server side for the themes.

@jgable
Copy link
Contributor Author

jgable commented Sep 14, 2013

Yeah, looks like it does from my testing. Felt like it was a little out of place in the middle of that initTheme method.

@ErisDS
Copy link
Member

ErisDS commented Sep 15, 2013

Something in this PR is failing on node 11 :(

@ErisDS
Copy link
Member

ErisDS commented Sep 15, 2013

This is kinda strange:

https://magnum.travis-ci.com/TryGhost/Ghost/builds/1217429 commit 0663308 worked
https://magnum.travis-ci.com/TryGhost/Ghost/builds/1217445 commit d67b3aa doesn't pass, have restarted 4/5 times

- Made a helper called ghostScriptTags that will spit out the relevant
  script tags with version parameter; 4 unminified files in development,
  1 minified file in production.
- Added grunt concat and uglify tasks to build files into core/built
- Fixed some unit tests by making them native date objects
@jgable
Copy link
Contributor Author

jgable commented Sep 15, 2013

Updated by taking out the express.compress() middleware. It looks like something is broken with it on 11.3. All the assets were being served with garbled contents.

@ErisDS
Copy link
Member

ErisDS commented Sep 16, 2013

This is neat, absolutely what I was looking for 👍

ErisDS added a commit that referenced this pull request Sep 16, 2013
@ErisDS ErisDS merged commit 1ee0d51 into TryGhost:master Sep 16, 2013
@JohnONolan JohnONolan deleted the gruntClientFiles branch September 16, 2013 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants