Fix compatibility with google closure advanced optimizations

[Skinney]

Problem

Crash or silent errors when compiled with google closure advanced optimizations.

Steps to reproduce

Compile the test suite with tests/run-tests.sh, take the resulting test.js script and compile it with google closure with advanced optimizations (can be done here). Run the result through node.

Why this commit fixes things

When compiled with advanced optimizations, the closure compiler will be so bold as to change the actual names of object properties. This means that an object like this:

{
    ctor: '_Tuple2',
    _1: 27,
    _2: "Robin"
}

Can end up looking like this after minification (pretty printed):

{
    a: '_Tuple2',
    b: 27,
    c: "Robin"
}

For the most part, this is just fine in the context of Elm. There were, however, certain places where explicit checks for the ctor property was made by using in checks, like this: if ('ctor' in obj) ....

The closure compiler will never change the contents of strings, so the result is that a few places, Elm will look for a ctor property which no longer exists, as it has been renamed.

This commit fixes the few places this caused a problem, and so this elm package should be compatible with google closures advanced optimizations.

How much does advanced optimizations matter?

On a hello world type Elm application, we get the following (without gzip):

232K    original.js
80K         uglify.js
76K         closure.simple.js
72K         closure.advanced.js

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[process-bot]

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

[evancz]

If you have Just "Just" this will be a problem.

[Skinney]

Right. Hmm. One other option is to always specify ctor as a string, both on creation and on retrieval, but that's a much bigger undertaking.

{'ctor': '_Tuple0'}
obj['ctor']

[evancz]

How comprehensively have you tested this? I'd be curious to know what grepping for "ctor" and 'ctor' in all code with native parts would yield. It's fine if you don't have it all, but I'd like to know what you did do this on at least.

[evancz]

How comprehensively have you tested this? I'd be curious to know what grepping for "ctor" and 'ctor' in all code with native parts would yield. It's fine if you don't have it all, but I'd like to know what you did do this on at least.

[eeue56]

this will fail if the key is equal to the value of the ctor. For example

{ ctor: "Ok"
, _0: "Ok" 
}

(same problem as brought up by evan, didn't see their comment til after!)

[Skinney]

I compiled a small application I've just started on and read/skimmed the code looking for constructs I knew closure would have a problem with, I then created a small benchmark to make sure I didn't seriously ruin performance (I didn't). I also grepped for 'ctor' and "ctor" just before submitting this, as a sanity check. So I think core and virtual-dom/html should be fine.

[Skinney]

I compiled a small application I've just started on and read/skimmed the code looking for constructs I knew closure would have a problem with, I then created a small benchmark to make sure I didn't seriously ruin performance (I didn't). I also grepped for 'ctor' and "ctor" just before submitting this, as a sanity check. So I think core and virtual-dom/html should be fine.

[evancz]

Cool, thanks for that info.

Independent from this, I am thinking of other ways of making assets as small as possible, so there may be some other better way. So before proceeding, I think it's worth quantifying the benefit that would come from this if we can make it work. So my questions are:

• How much smaller does this make things? If the minified and gzipped size is going from 24kb to 22kb, I'm not sure how invasive we should be to achieve that.
• Is the resulting code faster in any measurable way? If it's not faster, I'm also not sure how aggressively we should pursue this.

[evancz]

Cool, thanks for that info.

Independent from this, I am thinking of other ways of making assets as small as possible, so there may be some other better way. So before proceeding, I think it's worth quantifying the benefit that would come from this if we can make it work. So my questions are:

• How much smaller does this make things? If the minified and gzipped size is going from 24kb to 22kb, I'm not sure how invasive we should be to achieve that.
• Is the resulting code faster in any measurable way? If it's not faster, I'm also not sure how aggressively we should pursue this.

[Skinney]

I added some quick measurements on the bottom of the top post. The gzipped difference between simple and advanced optimizations is only 1-2kb. It's probably worth looking at a larger project though, as this is pretty close to a hello world type application.

Anyway, I'm not in any personal need to rush this, I just saw an opportunity to fix something people mention now and again with a small commit. Since my proposed solution here is flawed, I've no problems with putting a low priority on this one.

I'll try to compile my array test suite with advanced optimizations tomorrow, and see if it makes a performance difference.

[Skinney]

I added some quick measurements on the bottom of the top post. The gzipped difference between simple and advanced optimizations is only 1-2kb. It's probably worth looking at a larger project though, as this is pretty close to a hello world type application.

Anyway, I'm not in any personal need to rush this, I just saw an opportunity to fix something people mention now and again with a small commit. Since my proposed solution here is flawed, I've no problems with putting a low priority on this one.

I'll try to compile my array test suite with advanced optimizations tomorrow, and see if it makes a performance difference.

[evancz]

Okay, let's close this one for now then. We can open a new one if we figure out a safe way to do this / know the size and perf numbers on larger projects.

[evancz]

Okay, let's close this one for now then. We can open a new one if we figure out a safe way to do this / know the size and perf numbers on larger projects.