Create a minified browser build compatible with next.js #1108

jeduan · 2021-12-06T07:14:54Z

Browser Build

This PR creates a browser-specific build that includes none of the server-specific code.

It uses Rollup to create a single output that's tree-shakeable and that will not include node-specific APIs like NodeWallet and Provider.local(). The build is declared on package.json's browser field.

The code is split by individually reviewable commits

4e76e89 Updates buffer-layout to pick up browser fixes
4b6c594 and e57244b make sure server-only code is not added to the client bundle
0a05cb3 adds the rollup config per se

In addition a couple of goodies:

09b6df7 removes circular dependencies, which can make Typescript slower
101a717 refactors typescript configs into a base so the config is easier to use in rollup

How was this tested

I created a minimal next.js project which complained about fs and path being undefined on the client. After doing yarn link '@project-serum/anchor' and reloading, the project ran.

Fixes #244
See also #728 and #983

Related to #1073

Resulting bundle:

buffer-layout has a couple of browser-specific improvements in pabigot/buffer-layout#29 and pabigot/buffer-layout#27 that this update pulls in

* Separates NodeWallet into its own file * Ensures that NodeWallet and workspace are only exported on Node

Circular dependencies can make TypeScript compiling slower. In this commit we fix imports so things can be a bit faster.

A base config pattern will allow us to keep module emitting from typescript compiler options separate

This commit uses rollup, same as solana's web3 project uses to bundle a browser-specific build

jeduan · 2021-12-07T05:45:06Z

Updated to create ESM as per conversation on Discord. I also created a gist with the unminified build artifact in case y'all want to check it out.

The error making tests error out should be fixed now as well.

armaniferrante

Thank you for the thorough explanation! LGTM. Just a couple of nits.

ts/.gitignore

ts/src/nodewallet.ts

ts/src/utils/bytes/utf8.ts

igdeman · 2021-12-08T16:07:02Z

Hi guys, when we can expect updated npm package that includes this fix?

archseer · 2021-12-09T06:25:14Z

This change seems to have tripped up typescript-language-server for me. It's unable to see anchor.workspace anymore.

igdeman · 2021-12-09T09:40:42Z

I see there is a new version of npm package 0.19.0. Unfortunately looks like fix above produced a new bug. NextJS project compiles without any errors but on page load i'm getting this error:

SyntaxError: Cannot use import statement outside a module
at wrapSafe (internal/modules/cjs/loader.js:988:16)
at Module._compile (internal/modules/cjs/loader.js:1036:27)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1101:10)
at Module.load (internal/modules/cjs/loader.js:937:32)
at Function.Module._load (internal/modules/cjs/loader.js:778:12)
at Module.require (internal/modules/cjs/loader.js:961:19)
at require (internal/modules/cjs/helpers.js:92:18)
at Object.@project-serum/sol-wallet-adapter (/.../.next/server/pages/_app.js:219:18)
at webpack_require (/.../.next/server/webpack-runtime.js:33:42)
at eval (webpack-internal:///./utils/wallet-adapters/index.ts:8:91)
error - SyntaxError: Cannot use import statement outside a module
/.../node_modules/@project-serum/sol-wallet-adapter/dist/cjs/index.js:79
import EventEmitter from 'eventemitter3';

jeduan · 2021-12-09T16:06:19Z

@igdeman can you share your next js version and wether your project is using webpack 4 or 5? (Here’s how you’d know: https://nextjs.org/docs/messages/webpack5)

dominictwlee · 2021-12-10T15:29:22Z

This change seems to have tripped up typescript-language-server for me. It's unable to see anchor.workspace anymore.

Temporary workaround for this that worked for me was to create a types.d.ts file with:

import * as anchor from "@project-serum/anchor";
declare module "@project-serum/anchor" {
  export const workspace: any;
  export const Wallet: import("@project-serum/anchor/dist/cjs/nodewallet").default;
}

0xCryptoSheik · 2021-12-10T22:48:16Z

Hi,

The Wallet type & constructor was apparently useful even for a browser-target consumer context: #1122

I also have a couple of questions @jeduan:

Why minify the output bundle using Terser since, as you mention with Next.js example, this is for bundler consumers ?
Consumers can (do) minify themselves.
Why use Rollup (and output a bundle) instead of simply transpiling ?
Again, consumers can (do) bundle themselves.

If anchor is tree-shake-able with a rollup bundle, I don't see a reason why it wouldn't be just transpiled.

I understand the bundler helps with removing annoying imports of Node.js APIs that the consumer trivially nullify themselves (or that anchor can deal with differently, ie, by exposing a Node.js & a browser entrypoint) but I believe it's worth considering the benefits of not bundling a library, intended for bundling-capable consumers.

A bundler is totally fine for a CDN export like in #3 though it couldn't be the same build output configuration.

https://grep.app/search?q=anchor.Wallet

jeduan · 2021-12-10T23:53:56Z

@0xCryptoSheik

Why use Rollup (and output a bundle) instead of simply transpiling ?

We need a browser bundle because workspace.js is a module with side-effects. Tree shaking won't actually remove side-effects and the side effect it runs happens to import a node-only codepath. Here's the relevant docs in webpack.

Why minify the output bundle using Terser since, as you mention with Next.js example, this is for bundler consumers ?
Consumers can (do) minify themselves.

I'm curious on the downsides of minimizing library side.

The Wallet type & constructor was apparently useful even for a browser-target consumer context

the Wallet export pointed to NodeWallet https://github.com/project-serum/anchor/blob/105bb203c01af31e54b09e6816a0df949f132ef2/ts/src/index.ts#L7

but I believe it's worth considering the benefits of not bundling a library, intended for bundling-capable consumers

This project still publishes an ESM transpiled library in pkg.module pointing to dist/esm/index.js.

Having said that, I'm just a random contributor. You should feel empowered to send PRs 😄

0xCryptoSheik · 2021-12-11T00:23:42Z

We need a browser bundle because workspace.js is a module with side-effects. Tree shaking won't actually remove side-effects and the side effect it runs happens to import a node-only codepath. Here's the relevant docs in webpack.

Sounds like more than a bundler, we just need to exclude that module from the browser build output one way or another, or remove the side-effect, or tell the bundlers to treat the package as side-effects free.

I'm curious on the downsides of minimizing library side.

from the top of my head:

for the library package:
- more dependencies, higher maintenance cost, more complex & longer / resource-intensive build step, more risk of shipping broken code (untested & obscured)
for the consumers:
- no longer able (or much more difficult) to hack, transform, or patch the built files in node_modules, no readability, no control over the minification settings...

I see no upsides..

the Wallet export pointed to NodeWallet

yes and it was used in web projects nevertheless, now it's gone without a warning or a mention in the changelog :(

This project still publishes an ESM transpiled library in pkg.module pointing to dist/esm/index.js.

Yes, this is true, but bundlers such as Webpack will pick the browser key by default, and changing this isn't necessarily trivial bundler side + changing imports to @project-serum/anchor/dist/esm is likely to break without a warning sooner or later as well, since:

built files directory paths are often not considered as part of the API provided by the package and can change without a warning
this PR "anchors" the idea that the dist/cjs & dist/esm are Node.js-only, who knows what they'll include in a next release.

Having said that, I'm just a random contributor. You should feel empowered to send PRs smile

No problem, I'm more interested in finding a better solution to the initial problem.

igdeman · 2021-12-11T14:15:34Z

@igdeman can you share your next js version and wether your project is using webpack 4 or 5? (Here’s how you’d know: https://nextjs.org/docs/messages/webpack5)

Hi there, project is using nextjs version: 12.0.7. According to the link you provide, project should use webpack 5 as there is no webpack5: false flag in the config.

archseer · 2021-12-12T11:58:35Z

@dominictwlee nice, could this be upstreamed? Or is it just a workaround?

dominictwlee · 2021-12-12T18:20:29Z

@dominictwlee nice, could this be upstreamed? Or is it just a workaround?

Possibly, maybe not via a .d.ts file but adding those declarations directly in the index.ts file. I'll give it a try, will put up a PR shortly

SkyYap · 2022-11-07T20:42:54Z

Hi, I have viewed all the comments here and I am still confused on how can I use unpkg to export anchor to be used in browser. In @solana/web3.js we can choose to use the script below to be used in browser. Anyone can enlighten me which file should I locate so I can use Anchor as browser bundle?

<!-- Development (un-minified) -->
<script src="https://unpkg.com/@solana/web3.js@latest/lib/index.iife.js"></script>

<!-- Production (minified) -->
<script src="https://unpkg.com/@solana/web3.js@latest/lib/index.iife.min.js"></script>

jeduan added 8 commits December 5, 2021 21:46

Update buffer-layout

4e76e89

buffer-layout has a couple of browser-specific improvements in pabigot/buffer-layout#29 and pabigot/buffer-layout#27 that this update pulls in

Ensure node-only exports are only available in node environments

4b6c594

* Separates NodeWallet into its own file * Ensures that NodeWallet and workspace are only exported on Node

Remove circular dependencies

09b6df7

Circular dependencies can make TypeScript compiling slower. In this commit we fix imports so things can be a bit faster.

Refactor typescript configs into a base

101a717

A base config pattern will allow us to keep module emitting from typescript compiler options separate

Add browser build

0a05cb3

This commit uses rollup, same as solana's web3 project uses to bundle a browser-specific build

Add browser gating in Provider.local

e57244b

Change rollup to emit ESM

3a75bc3

Fix NodeWallet export

8362ec1

Remove reference to node-only TextEncode in browser build

e6e0843

armaniferrante reviewed Dec 7, 2021

View reviewed changes

ts/.gitignore Outdated Show resolved Hide resolved

ts/src/nodewallet.ts Show resolved Hide resolved

ts/src/utils/bytes/utf8.ts Outdated Show resolved Hide resolved

jeduan added 2 commits December 7, 2021 09:47

Address feedback

064c07b

Fix NodeWallet CJS / ESM compatibility

e610935

armaniferrante mentioned this pull request Dec 7, 2021

Provide browser compatible javascript package via CDN #3

Open

armaniferrante merged commit 0f8de4f into coral-xyz:master Dec 7, 2021

dominictwlee mentioned this pull request Dec 12, 2021

ts: add missing types for conditional exports #1137

Merged

archseer mentioned this pull request Jan 10, 2022

Update anchor to 0.20 smartcontractkit/chainlink-solana#91

Merged

Henry-E mentioned this pull request Nov 17, 2022

Remove terser for react-native support #2263

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a minified browser build compatible with next.js #1108

Create a minified browser build compatible with next.js #1108

jeduan commented Dec 6, 2021 •

edited

Loading

jeduan commented Dec 7, 2021 •

edited

Loading

armaniferrante left a comment

igdeman commented Dec 8, 2021

archseer commented Dec 9, 2021

igdeman commented Dec 9, 2021

jeduan commented Dec 9, 2021

dominictwlee commented Dec 10, 2021 •

edited

Loading

0xCryptoSheik commented Dec 10, 2021

jeduan commented Dec 10, 2021 •

edited

Loading

0xCryptoSheik commented Dec 11, 2021

igdeman commented Dec 11, 2021

archseer commented Dec 12, 2021

dominictwlee commented Dec 12, 2021

SkyYap commented Nov 7, 2022

Create a minified browser build compatible with next.js #1108

Create a minified browser build compatible with next.js #1108

Conversation

jeduan commented Dec 6, 2021 • edited Loading

Browser Build

How was this tested

jeduan commented Dec 7, 2021 • edited Loading

armaniferrante left a comment

Choose a reason for hiding this comment

igdeman commented Dec 8, 2021

archseer commented Dec 9, 2021

igdeman commented Dec 9, 2021

jeduan commented Dec 9, 2021

dominictwlee commented Dec 10, 2021 • edited Loading

0xCryptoSheik commented Dec 10, 2021

jeduan commented Dec 10, 2021 • edited Loading

0xCryptoSheik commented Dec 11, 2021

igdeman commented Dec 11, 2021

archseer commented Dec 12, 2021

dominictwlee commented Dec 12, 2021

SkyYap commented Nov 7, 2022

jeduan commented Dec 6, 2021 •

edited

Loading

jeduan commented Dec 7, 2021 •

edited

Loading

dominictwlee commented Dec 10, 2021 •

edited

Loading

jeduan commented Dec 10, 2021 •

edited

Loading