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

Reduce .jade.js bloat #1457

Merged
merged 6 commits into from
May 29, 2018
Merged

Reduce .jade.js bloat #1457

merged 6 commits into from
May 29, 2018

Conversation

strugee
Copy link
Member

@strugee strugee commented Oct 9, 2017

This takes a crack at only building .jade.js files when we need them. This means that package tarballs distributed on npm will be smaller, which is nice.

@coveralls
Copy link

coveralls commented Oct 9, 2017

Coverage Status

Coverage remained the same at 73.802% when pulling 6efcfbe on reduce-jade-js-bloat into d1037f2 on master.

@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage remained the same at 73.802% when pulling d0c6f95 on reduce-jade-js-bloat into d1037f2 on master.

@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage remained the same at 73.802% when pulling c0c3b1a on reduce-jade-js-bloat into d1037f2 on master.

@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage decreased (-0.1%) to 73.701% when pulling 739fdfb on reduce-jade-js-bloat into d1037f2 on master.

@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage decreased (-61.8%) to 12.009% when pulling 739fdfb on reduce-jade-js-bloat into d1037f2 on master.

@coveralls
Copy link

coveralls commented Oct 10, 2017

Coverage Status

Coverage increased (+0.06%) to 74.448% when pulling 45a8e0e on reduce-jade-js-bloat into 2ad16f7 on master.

@strugee strugee mentioned this pull request Nov 7, 2017
@strugee strugee self-assigned this Apr 4, 2018
@strugee
Copy link
Member Author

strugee commented Apr 4, 2018

IIRC when I worked on this I was pretty disappointed with the results (they didn't improve a lot) but I might as well merge this since it's been sitting open forever for no reason. Self-assigning to rebase and merge.

AFAICT this used to load dependency templates, but this is irrelevant
now that we use Jade's `include` which inlines dependencies
directly. So nuke this property from orbit. (Honestly, there didn't
seem to be anything referencing it anyway.)
These were picked up by a script when I originally created all the
`-client` files, but it's safe to get rid of these particular
`-client` splits because all these templates will only ever be
rendered by the server.

Because of the same reason, we move them to `lib/` so they don't get
rendered into client files.
@strugee strugee merged commit 45a8e0e into master May 29, 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.

2 participants