ably / ably-js

Javascript, Node, Typescript, React, React Native client library SDK for Ably realtime messaging service
https://ably.com/download
Apache License 2.0
316 stars 55 forks source link

RFC: Tree shaking #1210

Closed owenpearson closed 5 months ago

owenpearson commented 1 year ago

This RFC proposes the API and internal design for a tree-shakable ably-js client which will be included in version 2 of the library. Tree shaking is a form of dead code elimination used by module bundlers which works by statically analysis of ES6 import statements.

Tree shaking will allow users of the library to import features of ably-js independently, making it possible to import a 'minimal' ably-js client with only the necessary features. Since import size is more of a concern for the web browser environment, this proposal focusses on the ably-js web bundle, although the benefits will still be available to users on other platforms.

Constructor API

For backwards compatibility and convenience, the root import will still be an Ably object containing Rest and Realtime constructors (with all features included):

import Ably from '@ably/web';

const client = new Ably.Realtime(clientOptions);

Additionally, all other fields currently present on the Ably object will be importable directly:

import Message from '@ably/web/message';
import Rest from '@ably/web/message';
import Crypto from '@ably/web/crypto';

A base client will be exported which contains the minimal code for a functional ably-js client. Other modules will be exported separately and may be passed as in an object as a second argument to the BaseClient constructor:

import BaseClient from '@ably/web/baseclient';
import RealtimeSubscription from '@ably/web/realtimesubscription';
import RealtimePresence from '@ably/web/realtimepresence';

const client = new BaseClient(clientOptions, {
  RealtimeSubscription,
  RealtimePresence,
});

It is possible, using TypeScript generics, to dynamically change the SDK API depending on which fields are defined on the object passed in as the second argument to BaseClient so that, for example, if a user creates a BaseClient with no RealtimePresence then they won't get .presence as an autocomplete option on a RealtimeChannel instance (and of course, TypeScript won't compile code which tries to access such properties).

Modules available for tree-shaking

The ably-js automated bundle size report gives a helpful overview of how the web bundle is currently split in to modules. There are some cases where encapsulation of these modules is incomplete so the numbers are not precise, for example: realtimechannel.ts contains some presence related code, so just subtracting the size of realtime.presence.ts and presencemessage.ts won't give you a precise figure for a client without any realtime presence functionality.

This RFC proposes the following core modules will be available as tree-shakable imports:

Additionally, it is worth considering removing XHRRequest from the base client and making it optional with fetch as the default.

How much code would be eliminated?

A 'minimal' client is defined here as a realtime client which is able to subscribe to messages on a realtime channel. Currently, importing a v1 ably-js costs ~225kb. By inspecting modules from the bundle size report generated by 1ef4028aa7364089a74e80d9c1342378387c7f72 and calculating the total size of modules which would certainly not be included we would eliminate ~75kb.

Additional savings would come from:

It's difficult to estimate how significant the savings will be from the above list, but a rough estimate is that the resulting minimal client will be around 100-120kb.

┆Issue is synchronized with this Jira Task by Unito

sync-by-unito[bot] commented 1 year ago

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-3552

SimonWoolf commented 1 year ago

I don't fundamentally object to tree-shakability. But honestly the expected savings here seem not that exciting given the additional complexity to the user (and work to us)? 225kB down to 150kB. (Or 100-120 but that's including things like removing IE-compatibility code which would also benefit the non-treeshaked version, so not a fair comparison).

Might be worth flipping the order of things around? Like -- rather than doing tree-shaking first, then doing things like "Removing IE and ES3 compatibility code and some soft-deprecated APIs" to improve things further, why not start with the improvements that'll benefit everyone, then when that's done, reevaluate again to see how much then you'd get with tree-shaking ? ISTM there's quite a bit of heavy baggage in there we can cut quite easily -- eg if we can get rid of all that WordArray stuff now everyone's using browsers that support arraybuffer, that's a good 30kB right there (20kB of cryptojs imports and a bunch more in bufferutils & crypto simplification).

Talking of things to cut -- skimming through the file size history, in 1.2.4 we were at 179kB (minified); we jumped to 193kB with the es6 module conversion, then up to 220kB with the typescript conversion, which seems to have resulted in pages and pages of polyfills being added to the bundle, mostly for language features that we're not using (things like runtime support for transpiled async/awaits). ISTM worth reexamining what settings we transpile with to try and avoid the need for that?

lawrence-forooghian commented 1 year ago

I was wondering what this proposal would mean for our existing plugins mechanism, which also aims to "avoid excessively bloating the client library". Might we wish to consider removing the plugins mechanism and bringing the VCDIFF plugin into the ably-js codebase as an opt-in module?

lawrence-forooghian commented 1 year ago

I think that the proposal for splitting the library up into functional areas seems like a sensible one — the exact split I think depends on what our typical customer use cases look like and I guess @mikelee638 is best placed to answer that one.

I agree with @SimonWoolf that in terms of the order in which we do this work (i.e. tree shakability vs other stuff), it would make sense to start with stuff that is lower-hanging fruit and which impacts the largest number of users.

mattheworiordan commented 1 year ago

Thanks for this @owenpearson, there is some great thinking here.

Note: This post has been amended following @abhishiv pointing out my mistake of using Gzip sizes of other SDKs as opposed to raw minified size 🤦

My initial thoughts are:

Happy to have a call to discuss this, especially if we need something more radical, to explore what's possible.

jaley commented 1 year ago

I've had a look through the above suggestions and analysis. My 2c:

abhishiv commented 1 year ago

hey guys

I found this issue since I also don't like the fact that ably is larger compared to pusher/pubnub. But just wanted to say the factor is not 5-10x, since ably is 60kb gziped and pubnub is 45kb.

What I would ideally like is the ably protocol documented somewhere so i can connect to it directly via websocket - they way pusher has documented their protocol.

mattheworiordan commented 1 year ago

@abhishiv thanks for pointing out the mistake I made using Gzipped sizes when comparing against our minified byte size 🤦 I've updated my comment now to reflect your comments meaning we're in the ballpark of PubNub now in fact, without all the optimisation work we're doing which is encouraging!

In regards to the protocol documentation, we do in fact document it (it used to be part of our normal docs, but we moved it out as we found most devs weren't interested). See https://sdk.ably.com/builds/ably/specification/main/protocol/ and https://sdk.ably.com/builds/ably/specification/main/features/ for the complete spec.

VeskeR commented 5 months ago

This has been (mostly) implemented in ably-js v2 (too many PRs to link them all, so will mention them when necessary).

We now provide a modular variant of the library available at ably/modular path. We decided to stick with existing plugins mechanism to provide modular functionality for the library. Core plugins that are available as tree-shakable imports:

Regarding additional savings proposed by Owen in the original message:

crypto-js dependency also has been removed in https://github.com/ably/ably-js/issues/1239 as we now use Web Crypto API.

Also, next idea has not been implemented:

It is possible, using TypeScript generics, to dynamically change the SDK API depending on which fields are defined on the object passed in as the second argument to BaseClient so that, for example, if a user creates a BaseClient with no RealtimePresence then they won't get .presence as an autocomplete option on a RealtimeChannel instance (and of course, TypeScript won't compile code which tries to access such properties).

Instead we check available plugins at runtime and throw an error if user tried to use an API that requires a plugin which was not provided. Would we want to export this idea to a separate issue @owenpearson ?


@owenpearson @lawrence-forooghian Once above questions above are resolved, would you be happy to close this issue?

lawrence-forooghian commented 5 months ago

Instead we check available plugins at runtime and throw an error if user tried to use an API that requires a plugin which was not provided. Would we want to export this idea to a separate issue (…)?

I remember discussing this with Owen during the design of v2 — I think he made an explicit decision to go with the error-throwing approach, so I don't think anything further is needed here.

lawrence-forooghian commented 5 months ago

Removing code related to realtime publishing

I had a go at this in https://github.com/ably/ably-js/pull/1504 and we decided not to proceed with it since the bundle size reduction was disappointing. So I think we can forget about it.

lawrence-forooghian commented 5 months ago

I think we can close this issue.

VeskeR commented 5 months ago

Closing as implemented in ably-js v2.