feat(tree-shaking): enable browser target webpack tree-shaking

[0xCryptoSheik]

Hey, I'm throwing a few ideas at the tree-shaking issue of web3.js, would love some feedback, or some help testing / challenging this :)

Summary of Changes

• make an unbundled transpiled build using Babel

These files are meant to be consumed by Webpack in a way that will
enable tree-shaking.

• enable webpack tree-shaking by marking lib as side-effects free

Even though this is not exactly true, web3.js isn't strictly pure, it
should be safe.

See https://webpack.js.org/guides/tree-shaking/

Twitter thread: https://twitter.com/0xCryptoSheik/status/1453342552007057415

Only tested the most simplistic scenario with https://github.com/0xCryptoSheik/treeshake-solana-web3js

Fixes solana-labs/solana-web3.js#1122

/!\ Do not merge, needs thorough testing I can't do on my own.

[0xCryptoSheik]

Tried it in wallet-adapter-react-ui-starter, seems to work and bring fair enough gains.

Uncompressed

Before

• 2.13d77f2a.chunk.js 1364784 bytes / 1.36MB
• main.9ead91a1.chunk.js 72103 bytes / 72.1kB
• total 1436887 bytes / 1.44MB

After

• 2.1217a4d1.chunk.js 1229965 bytes / 1.23MB
• main.b53f84c0.chunk.js 157406 bytes / 157kB
• total 1387371 bytes / 1.39MB

Change

• total 49516 bytes / 49.5kB

gzipped

< 372.94 KB  build/static/js/2.13d77f2a.chunk.js
< 19.46 KB   build/static/js/main.9ead91a1.chunk.js
---
> 343.15 KB  build/static/js/2.1217a4d1.chunk.js
> 34.4 KB    build/static/js/main.b53f84c0.chunk.js

[0xCryptoSheik]

bump

[0xCryptoSheik]

I'm using this version in a web project with Webpack, FWIW, this is the required configuration:

  plugins: [
    new webpack.DefinePlugin({
      'process.env.BROWSER': true,
      'process.env.NODE_ENV': JSON.stringify(env),
    }),
  ],
  resolve: {
    fallback: {
      https: false,
      http: false,
    }
  },

The DefinePlugin constraint can easily be lifted and handled by Babel instead, similarily to what's done in the rollup configuration:

      replace({
        preventAssignment: true,
        values: {
          'process.env.NODE_ENV': JSON.stringify(env),
          'process.env.BROWSER': JSON.stringify(browser),
        },
      }),

[oJshua]

I'm using this version in a web project with Webpack, FWIW, this is the required configuration:

Thank you, I'll have another look today!

[0xCryptoSheik]

With the latest commit, the required webpack / consumer configuration is now:

  resolve: {
    fallback: {
      https: false,
      http: false,
    }
  },

The thing is, the following piece of code needs to be eliminated for browser builds:

src/connection.ts

import { AgentManager } from './agent-manager';
...
let agentManager: AgentManager | undefined;
if (!process.env.BROWSER) {
  agentManager = new AgentManager(useHttps);
}

So far I'm succesful in elimintating the process.env.BROWSER condition, but not the agent-manager import.

[0xCryptoSheik]

With the latest commit, I'm able to remove the dead import from the transpiled ESM output files, but I'm not too happy about it.

I also confirmed the tree-shaking still works as intended with a webpack consumer, and no longer requires any specific bundler configuration.

Thoughts?

[oJshua]

I'm testing out the latest commits, but wanted to bring in @jstarry since he's more familiar with this area.

[0xCryptoSheik]

Not really satisfied by this solution, forces the build to be aware that a specific import is unused in specific conditions ̛& needs to be removed.

I believe it might be best to not remove the import, and in fact ship the code containing references to http[s] Node.js module, even if it forces additional yet trivial configuration of consumers (which in most cases, is covered by frameworks build)

[0xCryptoSheik]

If we consider https://github.com/solana-labs/solana/pull/21055/files#r759707513, then this becomes unnecessary as well, the consumer's bundler / minifier will remove the bit of dead code:

  if (!process.env.BROWSER) {
    agentManager = new AgentManager(useHttps);
  }

[0xCryptoSheik]

This on the other hand, I think is good to keep in any case, since it enables not forcing consumers to inject themselves inlined process.env.BROWSER & process.env.NODE_ENV values in their builds (even though, process.env.NODE_ENV is injected by most bundlers automatically today)

[ZetiMente]

@0xCryptoSheik Is this also removing the node builtins so it will work on Webpack 5 and Vite?

Thank you for doing this!

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This stale pull request has been automatically closed. Thank you for your contributions.