cincheo / jsweet

A Java to JavaScript transpiler.
http://www.jsweet.org
Other
1.46k stars 160 forks source link

Use ES6 Set Implementation for java.util.Set #549

Closed iffyio closed 4 years ago

iffyio commented 4 years ago

When the target is ES6 it should be possible to use a native JS solution instead of an array. The indexOf search on every add/contains becomes debilitating once there are a large number of elements in the set.

I think this is fixable on an individual basis via a custom PrinterAdapter but I'm wondering if it should be feasible to cover it in JSweet instead? - A more general symptom seems to be that RemoveJavaDependenciesAdapter never takes into consideration what the target is? If so it might be good to make it target-aware.

lgrignon commented 4 years ago

Thanks for this nice suggestion. This is not possible out of the box, but did you notice there is an example for almost the same in the documentation

iffyio commented 4 years ago

Ah, I've just seen it ^^ I figured something custom like that would be doable for Sets but was hoping to have it built in instead. For example, if there is information about the target ES standard during substitution I imagine, with some refactoring, RemoveJavaDependenciesAdapter would be able to better support cases like this that can benefit from the newer target features?

lgrignon commented 4 years ago

I don't think it would be good for backward compatibility that RemoveJavaDependenciesAdapter changes its generated code. We could provide an option but I think it would be too specific to be available as a transpiler's option. Maybe the best solution would be to provide it as a built-in adapter on top of the RemoveJavaDependenciesAdapter that developers could use in their jsweetconfig.json : https://github.com/cincheo/jsweet/blob/master/doc/jsweet-language-specifications.md#installing-and-activating-adapters

iffyio commented 4 years ago

Ah good point. Having it as an add-on adapter makes a lot of sense then.