Saltarelle / SaltarelleCompiler

C# to JavaScript compiler – Now http://bridge.net
http://saltarelle-compiler.com
Other
297 stars 74 forks source link

Google Closure Compiler #90

Open JHemingway opened 11 years ago

JHemingway commented 11 years ago

The Google Closure Compiler gives some great compression on raw js files. The 'advanced mode' will strip out unused code etc and gives some great results, however it will break js code compiled with Saltarelle (ie registering a class).

Is there any chance you can support this?

Although it would be better that the Saltarelle compiler did this, I understand that it may not be a high priority. Using the Google Compiler might be more trivial to implement.

I am writing code that runs on mobile devices and I am feeling the need to have the js files as small as possible.

Thanks Joe

erik-kallen commented 11 years ago

How does it fail?

How much size reduction do you get compared to a Saltarelle release build?

JHemingway commented 11 years ago

FYI here is the tool: http://closure-compiler.appspot.com/home

The size reduction is significant, I reduced a Saltarelle release js file by 50%. In some cases it can be a lot more, for example using only a few methods in another assembly, the Google Compiler is smart enough to strip out all unused code.

The registerClass and registerNamepace cause issues (I am not sure if there is anything else, but I would doubt it). Using the strings that map to real objects will confuse it.

So MyNamespace.MyClass may get crunched to a.b, however the google compiler will not be able to connect this to the strings getting passed to the register methods.

a.b.registerClass('MyNamespace.MyClass');

erik-kallen commented 11 years ago

Does the problem still exist with the latest version (which uses the syntax ss.registerClass() instead of Namespace.Type.registerClass())?

I do not have any experience with the closure compiler, and I do not feel like learning it inside out in order to resolve this issue, so do you have any suggestion of how to change the generated script?

ProdigySim commented 11 years ago

I'm not seeing ss.registerClass() in the latest (1.6.2) version. Which branch is being used for this?

As far as I can tell, the real problem with being able to use Closure's advanced optimizations on Saltarelle code come from property names being accessed through strings. To be compatible with the closure compiler, you will need to "export" the names.

https://developers.google.com/closure/compiler/docs/api-tutorial3#export

I think perhaps the AMD pattern could help with this--but I don't really know in depth how this works.

Also note that this issue with using the closure compiler is one of the reasons I'm interested in Issue #84

erik-kallen commented 11 years ago

It might be Type.registerClass ( not by my computer now). To use AMD, use the ModuleName and AsyncModule attributes

JHemingway commented 11 years ago

Ok, I have had a look and I do not think there will be an easy way to implement this. The problem is that the class name is defined as a string and passed to Type.registerType. The Google Compiler will not see/understand this. It does not know that "MyNamespace.MyClass" is subString'd to create MyNamespace.MyClass.

erik-kallen commented 11 years ago

Does Closure have an option to not minimize names? Saltarelle already does the minimization. OTOH, given the above I wouldn't trust the dead code removal either (eg. will it cope with method overrides?)

JHemingway commented 11 years ago

I think method overrides will be ok as it references the method variable off of the prototype, so it is smart enough to not break this. Older versions of Script# used a string, this would have broken it however.

When using advanced minification it will always minimise names. It goes beyond Saltarelle in that it will crunch 'public' class names. It actually does more than just crunching names. It will also change the code where it thinks it can optimise it (ie it will inline code if appropriate). This is there hello world example:

function hello(name) { alert('Hello, ' + name); } hello('New user');

Gets converted to: alert("Hello, New user");

Usually I would stay away from minification that actually changes my code, but with a tool like Saltarelle rendering the js I think it is becoming safe to do so.

Regardless, Google Compiler will not work with how Type.registerType uses strings. Removing the strings doesn't look like it is an easy task. Namespaces, classes etc would all have to be created in the correct order.

I am happy to just have this feature sit on the wish list, as I imagine that you do not see this as a high priority.

erik-kallen commented 11 years ago

Ignore previous reference, it's just my bad.

erik-kallen commented 11 years ago

Looks like I will have to do something about the type system to fix #186. I can probably make a change that will fix this as well. Will you help me test a few different approaches? I have very little experience with Closure.

erik-kallen commented 11 years ago

@JHemingway Can you please try the latest 'develop' build and check if this works with Closure? (It will break the Linq bindings, though, so I have to adjust them if this works).

n9 commented 11 years ago

I've tried. The my testing project is not working with --compilation_level ADVANCED_OPTIMIZATIONS. I am trying to isolate what causes errors.

However, what I've hit lot of warnings: WARNING - dangerous use of the global this object (which does not seems to be related to the errors). The explanation to the warning says:

Note that the compiler only recognizes a function as a constructor if it is annotated with the @constructor JSDoc annotation.

What about to add this annotation to all constructors?

n9 commented 11 years ago

For some reason the root namespace export does not seem to work. (Although all javascript files are processed together.)

But maybe the problem is caused by current registration to global, so https://github.com/erik-kallen/SaltarelleCompiler/issues/194 might help the Closure compiler too.

erik-kallen commented 11 years ago

I don't like the idea of adding @constructor to constructors because IMO closure is just a tool among others and we can't add all metadata desired by all JS tools out there. Most likely, you can ignore all closure warnings, the C# analysis rules (from which the JS is generated) should be stricter than what closure can do.