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

Allow private class members to be mangled #12176

Merged
merged 1 commit into from Apr 8, 2021

Conversation

bz2
Copy link
Contributor

@bz2 bz2 commented Apr 3, 2021

Openlayers classes use an underscore suffix convention to mark class members as private. These are undocumented implementation details and presumably unsafe for code outside the class to rely on.

Allowing javascript minimisation to change these names gives 5% bundle size reduction to the library.

See terser docs on mangling property names and mange options.

Before/after size comparisons.

For common examples bundle:

929K build/examples/common.js
877K build/examples/common.js

After gzip -9:

246K build/examples/common.js.gz
241K build/examples/common.js.gz

And for legacy bundle:

985K build/legacy/ol.js
932K build/legacy/ol.js 

I've not exhaustively tested all examples, but from grep for '_ ' only examples/custom-interactions.js touches properties with an underscore suffix, and it's its own class.

This might be considered an api change, as it has some potential to break library users (if they're not building from the module sources themselves, and are poking around class internals), so may be worth a specific version bump of some kind.

Motivation for proposing this, is have been using this exact build setup when importing and building ol and having the same behaviour upstream makes the guarantee it'll work going forwards a bit stronger.

Openlayers classes use an underscore suffix convention to mark class
members as private. These are undocumented implementation details and
presumably unsafe for code outside the class to rely on.

Allowing javascript minimisation to change these names gives 5% bundle
size reduction to the library.
@tschaub
Copy link
Member

tschaub commented Apr 8, 2021

Thanks, @bz2. I don't think we would need to consider this a major (breaking) change as we only promise stability of our @api annotated methods and properties (in addition, the legacy build should not be used in production).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants