georgemandis / konami-js

Adding the Konami Code easter egg to your projects since 2009! Compatible with gestures on smartphones and tablets as well. Compatible with all front-end frameworks and vanilla JavaScript
http://konamijs.mand.is/
MIT License
959 stars 122 forks source link

Adding JSDoc annotations #16

Closed philipwhitt closed 10 years ago

philipwhitt commented 10 years ago

Added annotations for the google closure compiler.

sukima commented 10 years ago

Noob question: What value does the comment syntax offer? (I am not familiar with google closure).

philipwhitt commented 10 years ago

It enables the konami-js to be included by JavaScript apps that are compiled by the Google Closure compiler. Annotations are good to use regardless - shows your intent for a given segment code.

See http://goo.gl/wIQJ6i for the complete JSDoc annotation for the closure compiler.

sukima commented 10 years ago

I think I was more curious what feature a compiler uses for documentation annotations. Personally comments and annotations seem like personal preference and I was curious why the closure compiler imposes a documentation style.

sukima commented 10 years ago

The Closure Compiler looks for type information in JSDoc tags. Use the JSDoc tags described in the reference table below to help the compiler optimize your code and check it for possible type errors and other mistakes.

Ahh so closure is trying to turn JS into a more typed language like Java. That explains why the concept was so abrasive to me.

georgemandis commented 10 years ago

Doe this annotation actually result in a more optimal script when compiled with Closure?

Regardless, I'm inclined to keep konami.js compiler and package manager agnostic. I'm really only looking to merge pulls that can make the file size smaller.

I encourage you to fork the repo and start your own if you like!

philipwhitt commented 10 years ago

Yes, Google Closure Compiler does optimize the JS that goes through it, resulting in a smaller file. I suppose the title of this pull request was misleading, as JSDoc is compiler/package manager agnostic. Google Closure Compiler just happens to use JSDoc.

All I did was add a few simple JSDoc tags - resulting in a more accessible konami-js.

georgemandis commented 10 years ago

I ran the current incarnation of konami.js and your annotated version through the Closure Compiler UI (http://closure-compiler.appspot.com) and they both came in at the exact same size: 756 bytes gzipped and 1.73kb uncompressed. I tried running it in advanced mode with the annotated version thinking it might take advantage of your additions but it spit out an empty file.

If you can show me that this reduces the final output somehow I'd consider a merge, but if not I'm more keen to leave it. I do appreciate the pulls though!

philipwhitt commented 10 years ago

For advanced optimizations you have to export the name for external use. The simplest way to do that is window['Konami'] = Konami;

You then get an optimized version of the code totaling in 2kb... http://closure-compiler.appspot.com/code/jsc71af262d607735d118dd94f829fb49b5/default.js

The main reason I put in this pull request was to make it compatible with large, single page js apps that are compiled, using annotations, into a single file. For those who would include just the script itself on a more traditional web page, I'd suggest adding a konami.min.js to the project so they can benefit from the smaller file size.

philipwhitt commented 10 years ago

I added a konami.min.js to my fork, take a look and let me know what you all think:

https://github.com/philipwhitt/konami-js

Github says it's 1.502kb

philipwhitt commented 10 years ago

Still not convinced?

georgemandis commented 10 years ago

Forgot to follow up and check on this. I do appreciate the attention to detail you've provided, but I will have to pass on the merge. The Closure optimization tip via window['Konami'] was interesting though and we do get a really small file size. The annotations just don't strike me as particularly useful in helping to understand the code. I feel that by including them it's catering to a decidedly small subset of the people using this script.

I do genuinely appreciate it though!