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

[MOB-9520]: enable tree-shaking #451

Merged
merged 5 commits into from
Sep 12, 2024
Merged

[MOB-9520]: enable tree-shaking #451

merged 5 commits into from
Sep 12, 2024

Conversation

mprew97
Copy link
Contributor

@mprew97 mprew97 commented Sep 9, 2024

JIRA Ticket(s) if any

Description

Enables tree shaking to resolve this issue

Test Steps

  1. Remove sideEffects: false from package.json.
  2. Run yarn webpack-analyze from react-example dir
  3. Note the bundle size
  4. Add back sideEffects:false to package.json
  5. See smaller bundle size due to tree shaking

Before: 3.31 mb
Screenshot 2024-09-11 at 10 33 04 AM

After: 2.78 mb
Screenshot 2024-09-11 at 10 32 45 AM

@ts-nguyen
Copy link
Contributor

ts-nguyen commented Sep 10, 2024

I'm definitely not an expert with JS packages and bundle sizes but I would've expected changes in our webpack config files to reduce bundle size and for tree shaking. The changes here seem more like bundle stat info, is this just preliminary work? nvm actually looked at the issue and I see the change now, can you post a screenshot of the stat output to show the difference?

@lposen
Copy link

lposen commented Sep 10, 2024

@mprew97 -- one question -- are we importing any pollyfills or css? (not including things like style-components)

@lposen
Copy link

lposen commented Sep 10, 2024

@mprew97 Unless I'm doing something wrong, this hasn't changed the bundle size :( index.js remains at 353kb.

@mprew97
Copy link
Contributor Author

mprew97 commented Sep 11, 2024

@mprew97 Unless I'm doing something wrong, this hasn't changed the bundle size :( index.js remains at 353kb.

@lposen , @ts-nguyen added a screenshot with bundle sizes pre/post tree shaking.

My mistake, should have added that to begin with.

I also added the analyzer to the wrong project. Tree shaking shouldn't change the bundle size of the Web SDK itself, but rather the apps it is used in (from my understanding). So need to test in react-example

@mprew97
Copy link
Contributor Author

mprew97 commented Sep 11, 2024

@mprew97 -- one question -- are we importing any pollyfills or css? (not including things like style-components)

@lposen i do not see any explicit polyfills in our deps or webpack config. I do not think we are importing any external CSS either.

@mprew97 mprew97 merged commit 8405972 into main Sep 12, 2024
1 check passed
@mprew97 mprew97 deleted the MOB-9520 branch September 12, 2024 14:04
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.

4 participants