js-waku icon indicating copy to clipboard operation
js-waku copied to clipboard

Published 20 hours ago • verified publisher icon waku-org

Use proto library that provides tree-shakeable code

Open D4nte opened this issue 3 years ago • 4 comments

Problem

ts-proto generate code that is not tree-shakeable. See https://github.com/status-im/status-web/issues/233

This is an issue as it inflate the package size for library consumers.

It also uses Buffer.

Solution

Possibly investigate using ipfs/protons or protobuf-js directly.

Definition of Done

  • [ ] New library that generate tree-shakeable code is used

Bonus

js-waku currently uses bufbuild and ts-proto which has several downsides:

  • bufbuild is a separate binary that has to be installed
  • ts-proto generates code that uses Buffer It also has pros:
  • Generated code is TypeScript
  • Generates from proto files

Review whether we can use the solution for the examples in js-waku too.

D4nte avatar Nov 30 '21 03:11 D4nte

With protobufjs it's possible to build without bufbuild

https://github.com/filoozom/tfe-uliege-code/tree/main/platform%2Fnode

D4nte avatar Jan 29 '22 12:01 D4nte

I've also detected that generated files are not tree-shakeable, leaving a lot of unused code in the final bundle. See more: https://github.com/status-im/status-web/issues/233. I will investigate, whether there are some possibilities to optimize.

prichodko avatar Mar 02 '22 09:03 prichodko

Maintenance has been resumed :tada: https://github.com/ipfs/protons/commit/74d3b7abf1e857f7320c100734e797855ea728c1

Focusing this issue in the tree-shakeable problem.

D4nte avatar Apr 01 '22 02:04 D4nte

Protons is facing some performance issues: https://github.com/ipfs/protons/issues/51 The also haven't accepted the PR to align behaviour change with proto3: https://github.com/ipfs/protons/pull/48

Focus of this issue is still to have a tree-shakeable protbuf code. Whether we use protons, protobufjs (unlikely as it is cjs) or on own fork (of protons)

fryorcraken avatar Aug 25 '22 06:08 fryorcraken

Check https://buf.build/blog/protobuf-es-the-protocol-buffers-typescript-javascript-runtime-we-all-deserve

fryorcraken avatar Nov 01 '22 01:11 fryorcraken

Need to check if protons is tree shakeable also check this convo: https://github.com/ChainSafe/js-libp2p-gossipsub/pull/368#issuecomment-1299609972

fryorcraken avatar Jan 31 '23 06:01 fryorcraken

Do note https://github.com/status-im/status-web/pull/337

fryorcraken avatar Feb 14 '23 09:02 fryorcraken

Definition from https://github.com/vacp2p/waku are used

Most of the work done here: https://github.com/waku-org/js-waku/pull/1196

Peer exchange still pending: https://github.com/vacp2p/waku/issues/14

fryorcraken avatar Feb 27 '23 04:02 fryorcraken

Based on https://github.com/waku-org/js-waku/pull/1245, it can be deduced that protobuf-es while reducing the total lines of code by 50%, still increases the package size by about 13% which makes protons better suited?

cc @fryorcraken

danisharora099 avatar Mar 21 '23 10:03 danisharora099

Based on #1245, it can be deduced that protobuf-es while reducing the total lines of code by 50%, still increases the package size by about 13% which makes protons better suited?

cc @fryorcraken

Ok seems that in terms of bundle size, it does not help. Noting that this issue is specifically about bundle size (tree shakeable code) it would make sense to keep protons.

fryorcraken avatar Mar 27 '23 03:03 fryorcraken

Based on #1245, it can be deduced that protobuf-es while reducing the total lines of code by 50%, still increases the package size by about 13% which makes protons better suited? cc @fryorcraken

Ok seems that in terms of bundle size, it does not help. Noting that this issue is specifically about bundle size (tree shakeable code) it would make sense to keep protons.

Correct.

Would this mean we are closing this issue with the above findings?

danisharora099 avatar Mar 28 '23 06:03 danisharora099

As a followup to the above thread:

One idea that came up in a discussion with @felicio was if we could have the Logos scaling simulation tests run js-waku with both protobuf libraries and see some real-world statistics for sending/receiving messages at scale with both libraries.

Turns out it will take some work to make js-waku work with the Logos simulation test (ref: https://discord.com/channels/973324189794697286/1047940460791996446/1088787192870096978)

Curious to know your thoughts if it is worth it @fryorcraken

danisharora099 avatar Mar 31 '23 08:03 danisharora099

Yes this is a fair idea. But not for 2023 https://notes.status.im/Uz9HeCwZTDSYyOq36Q54cA?edit

I'd suggest to open a different issue and park it.

fryorcraken avatar Apr 03 '23 06:04 fryorcraken

Yes this is a fair idea. But not for 2023 https://notes.status.im/Uz9HeCwZTDSYyOq36Q54cA?edit

I'd suggest to open a different issue and park it.

Sounds good :)

Future work being tracked here: https://github.com/waku-org/js-waku/issues/1294

danisharora099 avatar Apr 03 '23 08:04 danisharora099