maryvilledev / codesplainUI

Web app for breaking down and annotating Python 3 source code.
https://www.codesplain.io
GNU General Public License v3.0
2 stars 2 forks source link

Optimize highlight.js imports #498

Closed solkaz closed 7 years ago

solkaz commented 7 years ago

Description

This PR refactors our imports from highlight.js, eschewing the default instance (which is around 520 kb when imported) and instead using a hand-rolled solution that supports only the Java and Python languages and is 12 kb.

Motivation and Context

To decrease our bundle sizes, the benefits of which should be obvious.

Checklist:

Browsers

Screenshots (if appropriate):

Before: Highlight.js bundled size: (520kb)

screen shot 2017-05-15 at 3 44 24 pm

Total bundle size: (~1.5mb)

screen shot 2017-05-15 at 3 44 34 pm

After: Highlight.js bundled size: (~12kb)

screen shot 2017-05-15 at 3 44 44 pm

Total bundle size: (~1mb)

screen shot 2017-05-15 at 3 44 54 pm
Hopding commented 7 years ago

I think this might remove too many languages. I think we should also support more of the common languages, specifically:

solkaz commented 7 years ago

Open to adding new languages, but we don't have users that are requesting those languages. Plus, it's easy to add a language; this was done to cut out the bloat.

thebho commented 7 years ago

Did you really try this on all the browsers ?

dane-johnson commented 7 years ago

@Hopding is right. People may want to write annotations with inlined code from a language we don't support, perhaps as a point of comparison. If bundle size is really a problem, we should find another way to reduce it.

solkaz commented 7 years ago

@thebho just on Chrome and Safari

thebho commented 7 years ago

Then why did you check them all?

solkaz commented 7 years ago

If we want to add more languages, then sure, we can do that later, and it's an easy process that's evident in the code. Right now, however, we don't have any users for Codesplain. We ourselves don't, and probably won't, use the annotation feature for other languages. That's why I don't feel a need to include more languages; if you really, really want a language, you're welcome to make a PR.

solkaz commented 7 years ago

@thebho because I imagined that this should work across all browsers. It's not a UI change. I have unchecked the ones that I haven't tested, though.

thebho commented 7 years ago

Just add the browsers you have actually tested the changes on. I'm not expecting every PR to have full coverage, but when there are layout or state changes, we should have a record of the browsers that have been tested

dane-johnson commented 7 years ago

Would like to see support for most of the top 30 http://spectrum.ieee.org/computing/software/top-10-programming-languages/

solkaz commented 7 years ago

@dane-johnson

if you really, really want a language, you're welcome to make a PR.