pusher/pusher-js

Bundle size

Closed this issue · 10 comments

This has been reported before but no comment by the team has been made. The bundle footprint is huge (100kB unzipped). Are there any plans to reduce this? Surely there is no need for such a massive footprint.

stale commented

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you'd like this issue to stay open please leave a comment indicating how this issue is affecting you. Thank you.

@amad @fbenevides Can we get an official response on this question?

Pusher is a paid product and it would be good to understand what is going on here to warrant 10s of kb of gzipped JavaScript in a frontend application, when the WebSocket API comes built-in.

amad commented

@odusseys @isaachinman
Have you tried the latest version? We recently reviewed our dependencies and pushed an update that slightly reduced the size.

Do you think it's still larger than it should be? Do you have any suggestions that could help us reduce the size?

https://js.pusher.com/8.1.0/pusher.min.js
Size: 61,8 kB
Actual transferred/download for client: 18,7 kB

https://js.pusher.com/8.1.0/pusher-with-encryption.min.js
Size: 94 kB
Actual transferred/download for client: 29,2 kB

@amad Thanks for the response. Yes, we're currently on v8.2.0.

The first place I'd look is your actual build pipeline. Not sure webpack is the right choice – something as simple as esbuild would probably suffice. But regardless, check the target here.

A UMD IIFE is not going to be treeshakeable. A good place to start might be adding a new ESM web target, eg:

import Pusher from 'pusher-js/esm'

We use Pusher in web settings (NextJs), NodeJs, and workers. We only use channels, and our use case is about as basic as could be. I imagine that given a shakeable target, a fair amount of unused code could be removed.

We are not using encryption, so our use case should exclude the tweetnacl dep, which is the only listed dep.

Happy to discuss further or work on a PR.

amad commented

@isaachinman It would be absolutely helpful if you could work on a PR. Otherwise I can add your suggestion to team's backlog for investigation.

@amad Spent about an hour looking through the repo and working on a fork. Some findings:

  • Most of the codebase seems to be very old and very outdated. There are even comments referencing todo fixups for TS v2
  • The quality of the codebase is low. Code is not organised well, actual classes are being exported from type declaration files, there are many unused imports, path aliasing is done via webpack not TS, mix of es3/es5, etc
  • There are quite a few polyfills and browser-compat considerations which are probably obsolete now and can be dropped. A lot of this was written 7+ years ago

It's surprising to see such a "legacy" codebase for an npm package doing >1m downloads per month, owned by a for-profit company, which is presumably driving a large portion of revenue/use cases. Even more surprising given that the total LOC seems to be <10k.

I was able to convert the Node and web targets to a treeshakable CJS bundles via esbuild, and saw improvements in our NextJs bundle sizes. However, properly "fixing" this repo would require a complete refactoring. I do think that the total LOC could be reduced by 30-50% with just a straight refactor, let alone improving the build pipeline.

Hope this is helpful, sorry to bring bad news. I won't invest anymore time personally on this, and will actually look at competitors as I am seriously not impressed.

amad commented

@isaachinman thanks for the feedback. I'll pass this to the team.

I will close this issue, thanks for the feedback.

@benjamin-tang-pusher Was any action taken? What's the reasoning for closing the issue?

@isaachinman its on our list of features we want to implement in the future, but as of now we have not taken specific action on this. We definitely want to make the pusher-js bundle size smaller, but we will likely do this at a later date.