Skip to content
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

Expand unit tests to cover all exported properties in minified code #579

Closed
brianchirls opened this issue Jun 18, 2013 · 5 comments
Closed

Comments

@brianchirls
Copy link

In "Advanced Optimizations" mode, Closure Compiler renames or removes certain properties and methods on objects. So many of the public methods and properties are renamed or not there at all. For example, techGet, techCall and requestFullScreen have been obfuscated. cancelFullScreen is not even available at all.

The API is rendered all but useless with the minified code. So, either advanced optimizations should be remove, or all public methods and properties need to be explicitly exported. I recommend the former to make sure you don't miss anything.

For reference: #165

@awkaiser
Copy link

As suggested, public methods and properties absolutely should be exported. There is no need to disable "advanced optimizations" if this is done.

@brianchirls
Copy link
Author

Agreed. It looks like the fullscreen issue has been addressed in #555, and there are unit tests for it. But there are still some missing properties, like the tech-related ones mentioned above, that are useful and were available in previous versions as well as the current un-minified version.

So I suggest, instead the following:

  • that the unit test be expanded to cover all documented API methods/properties
  • previously exported methods/properties be reviewed for inclusion in exports and documentation
  • and, of course, that exports.js be updated to fix whatever items are missing from the above

I will change the title of this issue accordingly and, if time allows, try to review which properties I think should be included.

@heff
Copy link
Member

heff commented Jun 26, 2013

I started on this and there's a test that checks for certain API methods. If anything's missed feel free to add to it. Really anything that's already in exports.js should be listed there as well, so there's definitely more to add.

@onyxrev
Copy link
Contributor

onyxrev commented Aug 7, 2013

Yes, I forgot when I open-sourced the resolution switching plugin that it relies upon a bunch of fairly low-level API methods to do its work and won't function until they're de-obfuscated. I'll see if I can come up with a list of methods and properties needed to support the plugin.

@heff
Copy link
Member

heff commented Mar 5, 2014

We've exported many more methods since this issue, so I'm going to close it, but feel free to mention any others that are needed. #637 has a few that are outstanding.

@heff heff closed this as completed Mar 5, 2014
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants