Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to esbuild #79

Closed
L-as opened this issue Feb 7, 2022 · 16 comments · Fixed by #1521
Closed

Switch to esbuild #79

L-as opened this issue Feb 7, 2022 · 16 comments · Fixed by #1521
Assignees
Labels
enhancement New feature or request

Comments

@L-as
Copy link
Member

L-as commented Feb 7, 2022

I noticed #62 , and you haven't considered esbuild seemingly.

https://esbuild.github.io/

This is the best bundler I know of.

@klntsky
Copy link
Member

klntsky commented Feb 7, 2022

We need asynchronous Wasm loading. We need to check if it supports that.

UPD: it does.

@ngua ngua added the enhancement New feature or request label Feb 23, 2022
@ngua ngua changed the title esbuild? Switch to esbuild Mar 18, 2022
@ngua ngua self-assigned this Mar 18, 2022
@ngua
Copy link
Contributor

ngua commented Mar 18, 2022

Webpack is having some issues now so I think it's time to switch

@ngua ngua removed their assignment Mar 19, 2022
@ngua
Copy link
Contributor

ngua commented Mar 19, 2022

Well, esbuild might not be a viable choice. It's ability to load WASM has some limitations, and attempting to use it results in:

✘ [ERROR] Top-level await is currently not supported with the "cjs" output format

    wasm-module:/nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib_bg.wasm:73:44:
      73 │         export const { instance, module } = await loadWasm(wasmModule, imports);
         ╵                                             ~~~~~

✘ [ERROR] This require call is not allowed because the transitive dependency "wasm-module:/nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib_bg.wasm" contains a top-level await

    output/Serialization.Address/foreign.js:5:26:
      5 │     CardanoWasm = require('@ngua/cardano-serialization-lib-browser');
        ╵                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  The file
  "../../../../nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib.js"
  imports the file
  "wasm-module:/nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib_bg.wasm"
  here:

    ../../../../nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib.js:1:22:
      1 │ import * as wasm from "./cardano_serialization_lib_bg.wasm";
        ╵                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  The top-level await in
  "wasm-module:/nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib_bg.wasm"
  is here:

    wasm-module:/nix/store/9x9839hi8fncj4z187kgck6dyngs6f45-node-dependencies-realpab-1.0.0/lib/node_modules/@ngua/cardano-serialization-lib-browser/cardano_serialization_lib_bg.wasm:73:44:
      73 │         export const { instance, module } = await loadWasm(wasmModule, imports);
         ╵                                             ~~~~~


@klntsky
Copy link
Member

klntsky commented Mar 19, 2022

@ngua We can try parcel (it says it supports async WASM loading).

@klarkc
Copy link

klarkc commented May 22, 2022

@jy14898
Copy link
Contributor

jy14898 commented Aug 11, 2022

Another alternative, which is recommended by vites docs https://github.com/Menci/vite-plugin-wasm.

@Banou26
Copy link

Banou26 commented Sep 9, 2022

Well, esbuild might not be a viable choice. It's ability to load WASM has some limitations, and attempting to use it results in:

If #400 gets done, this won't be an issue anymore.
As this is tied to purescript 0.14 outputting CJS, which prevents true TLA from working, purescript 0.15 supports ESM outputs.

@Banou26
Copy link

Banou26 commented Sep 12, 2022

Meanwhile, for people that don't wanna wait for CTL to upgrade to Purescript 0.15, using Vite(which uses esbuild & rollup under the hood) which support TLA via plugins, you're able to import CTL builds
By CTL builds i mean the webpack output that CTL produces, i have not managed to move CTL to actually replace webpack with Vite as i know pretty much nothing about Nix. But it would definitely be possible for anyone knowing what they're doing with Nix.

You'll need to use these plugins @esbuild-plugins/node-globals-polyfill, @esbuild-plugins/node-modules-polyfill , rollup-plugin-node-polyfills(for prod) & vite-plugin-wasm to polyfill/support CTL's requirements

I've managed to cut back build times by more than 80-90% in most cases

If you know what you're doing with Nix and would like to open a PR to move CTL to Vite but can't manage to get Vite working with purescript's output, I'd be more than happy to help out
But the best possible way to support all bundlers is still to move to Purescript 0.15. So doing that would be better than just moving to Vite instead of webpack.

@klarkc
Copy link

klarkc commented Sep 20, 2022

I managed to get CTL working with purs-nix, as a dependency. Purs-nix uses esbuild to bundle it, but I'm struggling with the wasm:

✘ [ERROR] No loader is configured for ".wasm" files: ../../nix/store/7kwphzh7r9a2zrvdj2f4xdz7k2rikwfd-cardano-transaction-lib-2.0.0-alpha/node_modules/@emurgo/cardano-serialization-lib-browser/cardano_serialization_lib_bg.wasm

    ../../nix/store/7kwphzh7r9a2zrvdj2f4xdz7k2rikwfd-cardano-transaction-lib-2.0.0-alpha/node_modules/@emurgo/cardano-serialization-lib-brows
er/cardano_serialization_lib.js:1:22:
      1 │ import * as wasm from "./cardano_serialization_lib_bg.wasm";

Well, esbuild might not be a viable choice. It's ability to load WASM has some limitations, and attempting to use it results in:

@ngua @Banou26 did you get the loader working? What do you mean by true TLA? (EDIT: I got it, TLA is top level await)

An esbuild plugin is the recommended solution, but nix esbuild just have the CLI, It seems that I'll need to make a derivation to bundle the project with the wasm plugin loaded, even so it will not solve TLA issues 🤔

@klarkc
Copy link

klarkc commented Sep 23, 2022

I tried to use this bundle script in the Scaffold project output (from purescript compiler):

#!/usr/bin/env node
// bundle.mjs
import { build } from 'esbuild';
import { NodeGlobalsPolyfillPlugin as nodeGlobals } from '@esbuild-plugins/node-globals-polyfill';
import { NodeModulesPolyfillPlugin as nodeModules } from '@esbuild-plugins/node-modules-polyfill';
import wasm from 'esbuild-plugin-wat';

build({
	entryPoints: ['result/Main/index.js'],
	bundle: true,
	outfile: 'index.js',
	define: {
		BROWSER_RUNTIME: "true",
	},
	plugins: [nodeGlobals(), nodeModules(), wasm()],
});

It throws in the browser:

Uncaught TypeError: (void 0) is not a function
    at BigNum.zero (index.js:4839:29)
    at ../../../../../../../nix/store/z8fq1kjac0akh2859c5r9ipqy0gyaaz0-Main-compiled/Types.BigNum/foreign.js (index.js:60150:33)
    at __require2 (index.js:18:52)
    at ../../../../../../../nix/store/z8fq1kjac0akh2859c5r9ipqy0gyaaz0-Main-compiled/Types.BigNum/index.js (index.js:61958:22)
    at __require2 (index.js:18:52)
    at ../../../../../../../nix/store/z8fq1kjac0akh2859c5r9ipqy0gyaaz0-Main-compiled/FromData/index.js (index.js:64348:26)
    at __require2 (index.js:18:52)
    at ../../../../../../../nix/store/z8fq1kjac0akh2859c5r9ipqy0gyaaz0-Main-compiled/Serialization.Address/index.js (index.js:66683:22)
    at __require2 (index.js:18:52)
    at ../../../../../../../nix/store/z8fq1kjac0akh2859c5r9ipqy0gyaaz0-Main-compiled/Address/index.js (index.js:67970:35)

I looked at this part of the code, probably relates to the wasm loader. I don't know anything about wasm, so I am leaving it here, maybe someone might find it useful.

@klarkc
Copy link

klarkc commented Sep 23, 2022

Just to add to my comment #79 (comment), and #79 (comment)

Vite wont work until #400 gets done, because the only compatible commonjs plugin move imports to top-level, skipping BROWSER_RUNTIME evaluation and creating more issues with node imports in browser context.

As far as I know, currently, the only option to bundle CTL remains webpack.

@Banou26
Copy link

Banou26 commented Sep 24, 2022

I am pretty sure this isn't true, Vite doesn't need any third party plugins to load CJS and is able to use both ESM's(True) and CJS's(Fake) TLA.

Here's my vite config if you wanna try for yourself (you can replace my-ctl-project with whatever purescript is outputting and it should work)

import { defineConfig } from 'vite'
import { NodeGlobalsPolyfillPlugin } from '@esbuild-plugins/node-globals-polyfill'
import { NodeModulesPolyfillPlugin } from '@esbuild-plugins/node-modules-polyfill'
import rollupNodePolyFill from 'rollup-plugin-node-polyfills'
import wasm from "vite-plugin-wasm"

export default defineConfig({
  resolve: {
    alias: {
      util: 'rollup-plugin-node-polyfills/polyfills/util',
      sys: 'util',
      events: 'rollup-plugin-node-polyfills/polyfills/events',
      stream: 'rollup-plugin-node-polyfills/polyfills/stream',
      path: 'rollup-plugin-node-polyfills/polyfills/path',
      querystring: 'rollup-plugin-node-polyfills/polyfills/qs',
      punycode: 'rollup-plugin-node-polyfills/polyfills/punycode',
      url: 'rollup-plugin-node-polyfills/polyfills/url',
      string_decoder: 'rollup-plugin-node-polyfills/polyfills/string-decoder',
      http: 'rollup-plugin-node-polyfills/polyfills/http',
      https: 'rollup-plugin-node-polyfills/polyfills/http',
      os: 'rollup-plugin-node-polyfills/polyfills/os',
      assert: 'rollup-plugin-node-polyfills/polyfills/assert',
      constants: 'rollup-plugin-node-polyfills/polyfills/constants',
      _stream_duplex:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/duplex',
      _stream_passthrough:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/passthrough',
      _stream_readable:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/readable',
      _stream_writable:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/writable',
      _stream_transform:
        'rollup-plugin-node-polyfills/polyfills/readable-stream/transform',
      timers: 'rollup-plugin-node-polyfills/polyfills/timers',
      console: 'rollup-plugin-node-polyfills/polyfills/console',
      vm: 'rollup-plugin-node-polyfills/polyfills/vm',
      zlib: 'rollup-plugin-node-polyfills/polyfills/zlib',
      tty: 'rollup-plugin-node-polyfills/polyfills/tty',
      domain: 'rollup-plugin-node-polyfills/polyfills/domain'
    }
  },
  define: {
    BROWSER_RUNTIME: true,
    process: {}
  },
  build: {
    target: 'esnext',
    commonjsOptions: {
      include: [/my-ctl-project/, /node_modules/],
      transformMixedEsModules: true
    },
    rollupOptions: {
      external: ['rollup-plugin-node-polyfills', 'util']
    }
  },
  optimizeDeps: {
    include: ['my-ctl-project'],
    esbuildOptions: {
      plugins: [
        NodeGlobalsPolyfillPlugin({
          process: true,
          buffer: true,
          define: {}
        }),
        NodeModulesPolyfillPlugin()
      ]
    }
  },
  plugins: [
    wasm(),
    rollupNodePolyFill()
  ],
  server: {
    fs: {
      allow: ['..']
    }
  }
})

@klarkc
Copy link

klarkc commented Sep 24, 2022

@Banou26 I have no idea why, but it keeps not transforming CJS modules (ReferenceError: require is not defined), which vite version are you using? For some weird reason optimizeDeps.include seems to not be taking effect, I tried variations of just putting the output (which is purs result) and creating a package.json inside it, absolute path to output/Main/index.js, nothing takes effect.

Here is a repro repo: https://github.com/klarkc/test-vite

nox vite build - ok it works
npx vite - no dev runtime

@Banou26
Copy link

Banou26 commented Sep 24, 2022

Here's your repo fixed https://github.com/Banou26/cardano-transaction-lib-issue-79
include: ['my-ctl-project'], only works for dependencies AFAIK, and you passed your CJS code as actual entrypoint, so it doesn't get optimised
Simple solution is to link the purescript output as temporary package and wrap it around whatever you want, in my case i made a simple ESM typescript src/index.ts entrypoint file to wrap.

@klarkc
Copy link

klarkc commented Feb 2, 2023

I've managed to fix the TLA error with vite by using the exclude option of optimizeDeps, beyond including my-ctl-project:

  optimizeDeps: {
    include: [
      "my-ctl-project",
    ],
    exclude: [
      "@emurgo/cardano-serialization-lib-browser",
      "@emurgo/cardano-message-signing-browser",
    ],
  },

Because include was adding everything in my-ctl-project, including its dependencies, it was trying to prebundle these libraries which were already es modules (so it does not need to be pre-bundled by vite). This pre-bundling were triggering the TLA error. Of course it also means that to run the dev environment one will need to use a TLA compatible browser.

@klntsky
Copy link
Member

klntsky commented Sep 21, 2023

Implemented here:

#1521

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

7 participants