New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Esprima minification corrupts some lambdas #64

Open
opichals opened this Issue May 9, 2017 · 10 comments

Comments

Projects
None yet
2 participants
@opichals
Copy link
Contributor

opichals commented May 9, 2017

Please have a look at the espruino/EspruinoDocs@25dc728 MPR121 sensor module commit for the workarounds (removed the use of lambdas in favor of functions) that had to be made for the module to get minified correctly.

@gfwilliams

This comment has been minimized.

Copy link
Member

gfwilliams commented May 10, 2017

Do you have an example of what way they got corrupted? I can't put minification into ES6 mode because Espruino doesn't support the full ES6 spec and the output would almost certainly contain code that Espruino couldn't execute. I guess that might affect the type of code that minification can accept?

If so, this might just be something we have to live with.

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented May 10, 2017

I think it works when the exports.connect is lambda, but the arguments are not minified at all.

When the MPR.setThresholds or MPR.touched is lambda the module gets truncated strangely so that it is not functional.

Do we know what are we missing from ES6 that could confuse the minifier? It could be a good idea to complete the Espruinos' ES6 support to whatever level is necessary.

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented Oct 30, 2017

@gfwilliams

This comment has been minimized.

Copy link
Member

gfwilliams commented Oct 30, 2017

I'm open to suggestions for workarounds. Sadly even Esprima (the tool used for offline minification in the Web IDE) doesn't seem capable of handling them :(

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented Oct 30, 2017

I am not sure about all the requirements. The new kid on the block for ES6 minification appears to be https://github.com/babel/minify. It is quite slow compared to say Uglify, but it should be easy enough to create an Espruino related Babel minify plugin stew.

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented Dec 19, 2017

The closure compiler is reported to work in node as well as in the browsers so once the EspruinoTools lib packaging is resolved it should just be fine. See my experiment working in node.js.

This could be a reasonable solution. Or am I missing something?

@gfwilliams

This comment has been minimized.

Copy link
Member

gfwilliams commented Dec 19, 2017

There's the online closure minifier that should work then?

My problem is that while I could drag it into EspruinoTools, the chances of actually getting that running in the browser for the graphical IDE seem slim?

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented Dec 19, 2017

It should just work IMO. They use GWT which produces browser compatible JS output. I'll try to do a proper PR in this direction.

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented Dec 13, 2018

This should not be a problem as there is terser minification now available for offline minification.

@opichals opichals changed the title Module minification corrupts some lambdas Esprima minification corrupts some lambdas Dec 13, 2018

@opichals

This comment has been minimized.

Copy link
Contributor Author

opichals commented Dec 13, 2018

The #58 (or espruino/Espruino#1078) seems to be a duplicate of this.

@gfwilliams I'd suggest deprecating the Esprima minification in favor of terser as soon as the EspruinoTools get release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment