search

chartist-js / chartist

Simple responsive charts
https://chartist.dev
MIT License
13.35k stars 2.53k forks source link

Invalid use of the super keyword breaks functionality in some cases. #455

Closed braaibander closed 8 years ago

braaibander commented 9 years ago

In the code there are quite a few usages of the super keyword as an identifier. Since super is one of the future reserved keywords, it should not be used as identifier (@see spec).

Most browsers will just accept this without even giving an error, but several tools (for instance, YUI compressor, or in my case, a 3rd party PDF printing service) are more strict on the JS specification and therefore just fail (one more graceful then the other).

I forked your repo and tried to fix it, but I am not particularly experienced with node.js and the setup and steps it requires, so after following your instructions for local deploy I was confronted with a lot of 404 errors on most JS files, which kind of stopped me in my tracks and forced me to just fix it in the dist folder. Just FYI

gionkunz commented 9 years ago

You are only partially correct about this. We have already discussed this several times. If you read the specs carefully, http://www.ecma-international.org/ecma-262/5.1/#sec-11.1.5 clearly states that an object property during initialization is a IdentifierName. Also looking at accessing properties here http://www.ecma-international.org/ecma-262/5.1/#sec-11.2.1 shows that you can write MemberExpression . IdentifierName to access properties.

If you read about IdentifierName here http://www.ecma-international.org/ecma-262/5.1/#sec-7.6 , you'll find that it can be any character sequence (including unicode sequence, $ and _) but not start with a number.

A reserved word is an IdentifierName that cannot be used as an Identifier.

Reserved words are perfectly fine as ItentifierNames but they are not as Identifier and therefore can't be used where Identifiers are expected (declarations).

Identifier :: IdentifierName but not ReservedWord

Therefore, writing var obj = {class: {var: {let: {super: { function: 10}}}}} and obj.class.var.let.super.function is perfectly fine. I'd not recommend to write this as it can be totally confusing. In the case of Chartist though it makes sense because it's an early emulation of classes and the super property has the same meaning as the super keyword in ES6.

Also I'd not describe the super keyword as a reserved keyword of the future. It's in ES6 which we all should use in our day to day job.

Finally, please stop using YUI Compressor. It's way outdated and YUI Compressor is no longer maintained years ago if I'm not mistaken and even the YUI team is no longer existing (with that name at least). Please use modern, far more progressed tools like Google closure compiler, uglify JS or the like.