Skip to content

Tree-Shake ES5 classes #2829

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

Closed
wants to merge 3 commits into from
Closed

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Mar 30, 2020

Rollup doesn't tree-shake ES5 classes since it cannot detect that they are side-effect free. The recommended way to solve this is to not use Typescript for the ES5 conversion, but Babel. Unfortunately, Typescript's ES5 is much smaller than Babels.

Luckily, the Internet answers all questions:

Typescript emits "@Class" comment when a class is side-effect free:
microsoft/TypeScript#13721

This can be rewritten to PURE as show here:
rollup/rollup#2807 (comment)

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 30, 2020

Binary Size Report

Affected SDKs

SDKTypeBase (74b06f5)Head (be794bf)Diff
@firebase/firestore/memorybrowser226315.00216317.00-9998.00 (-4.42%)
module223495.00213647.00-9848.00 (-4.41%)
main400263.00380273.00-19990.00 (-4.99%)
@firebase/firestorebrowser272888.00273245.00+357.00 (+0.13%)
module269667.00270024.00+357.00 (+0.13%)
esm2017269667.00217922.00-51745.00 (-19.19%)
firebasefirebase.js847447.00847467.00+20.00 (+0.00%)
firebase-firestore.memory.js268620.00258611.00-10009.00 (-3.73%)
firebase-firestore.js313861.00313881.00+20.00 (+0.01%)
Metric Unit: byte

Test Logs

@schmidt-sebastian schmidt-sebastian changed the title Base all Firestore builds on ESM2017 Tree-Shake ES5 classes Mar 30, 2020
@Feiyang1
Copy link
Member

Feiyang1 commented Mar 30, 2020

Nice. Can you please check if source maps work correctly? I have used this workaround when doing size analysis. The size analysis tool told me the source maps are invalid where the number of lines mismatch, so I just switched to use es2017 builds for analysis.

@schmidt-sebastian
Copy link
Contributor Author

Yeah, the source maps are not working... Looking into it.

@schmidt-sebastian schmidt-sebastian deleted the mrschmidt/betterbuilds branch April 6, 2020 19:26
@firebase firebase locked and limited conversation to collaborators May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants