Provide an ESM build

[kevinramharak]

Hi,

I'm trying to use this library for a vscode extension I am working on. I am using esbuild to build my extension, and it is able to bundle code from different module types (esm, cjs, umd) by using package entry points.

Now I noticed the code at index.js is written in CJS format but no ESM build is distributed.

Looking at the scripts/build.js esbuild is already used. As far as I can tell, additional build configurations can be added to add an ESM build. By using exports in package.json this would allow modern ESM imports to resolve to the ESM format instead of the legacy CJS format, without any changes to current consumers of the CJS format.

Are you open to either implement another bundle that distributes a ESM build or accepting PR's that do?

[kevinramharak]

It took me some effort to get the project building, as I never touched a line of scala, but I have figured it out now. Creating ESM builds is very easy to add, mostly just add an index.mjs with:

export { check, checkSync } from './lib/main.mjs';

have the builds run for both esm and cjs formats and adjust the package.json:

  "main": "index.js",
  "browser": "lib/browser.js",
  "exports": {
    "node": {
      "types": "./index.d.ts",
      "import": "./index.mjs",
      "default": "./index.js"
    },
    "browser": {
      "types": "./index.d.ts",
      "import": "./lib/browser.mjs",
      "default": "./lib/browser.js"
    }
  },

This is enough to allow consumers to import an ESM build instead of being forced to consume the CJS build.

---

I also found that the scalajs build is already in ESM format although its not using splitting of modules.
By adding the following to the ScalaJS config:

  // ScalaJS config:
  scalaJSLinkerConfig ~= {
    _.withModuleKind(ModuleKind.ESModule)
      // use smallest modules
      .withModuleSplitStyle(org.scalajs.linker.interface.ModuleSplitStyle.SmallestModules)
  },

This enables esbuild (and other bundlers) to see this:

Instead of the big blob that is currently used:

With the current structure of the JS API this has no effect, but it might with the changes mentioned below.

---

That being said, there is no actual gain because of the way the project is structured. It's very forgiving and provides environment variable detection to figure out which backend to use. Also the worker seems to be a duplicate of the codebase, but as a string to be able to run the code in a worker, which balloons the (very large) size of 1.4mb to 2.9mb.

I think this can be easily avoided by proving an (additional) API that allows you to explicitly import the backend and create a check function that way. Many modern javascript projects have a structure like that, for example Shiki. They still provide a simple API that users can import to have all features included, but also an API that allows to build a highlighter without lots of vendored assets that the end user knows they will not use, hugely decreasing bundle size.

Something like:

import { backend } from 'recheck/backend/native'
import { createCheck } from 'recheck/core'

const check = createCheck(backend);

with the correct structure could already shed of all half of the size, because the whole worker blob can be dropped.

While internally recheck can still provide the same API as before, just using the same core functions that are exposed to users to build their own API.

---

I'm still prototyping a fork to see what the gains can be exactly, but I think this could greatly improve the usability if you consider this proposal.

[makenowjust]

I agree with enabling the ESM build. It seems easy, and today ESM is more popular than CJS.

However, splitting codes like Shiki for size reduction sounds not good to me. This complicates the build process for this library, and makes it difficult for users to use this library with specific bundlers.

I have a limited amount of time to maintain this library, so I prefer not to make the maintenance harder.

Two years ago, when I implemented the worker, there was no standard way to split worker codes. Therefore, to make it easy for anyone to use the library with some bundlers, I decided to embed the worker code inline. If we want to implement code splitting, we will need to add and maintain documentation on how to build it with various bundlers, ensuring it remains as easy to use as it is now.

[kevinramharak]

@makenowjust I appreciate the response, and also really appreciate this library. I think this fills a gap in the JS ecosystem as regex dos can be a very serious problem. I'm hoping to contribute to allow for easier adoption, as the current bundle size makes it hard to justify its use. I was already looking for an alternative when I saw the cost of using the library, but figured I would first try and help out to optimize it.

I fully understand the concern about maintenance, I will try to show a prototype of what it entails, as it is not without cost. I am pretty close to finishing it, and I think the results are worth some of the pain changes will introduce, but that will be up to you to decide when you see it. We can mostly blame the JS ecosystem for making modules and CJS/ESM conflicts so hard to implement and migrate properly for library authors.

As for making it harder on consumers, I think you misunderstood what I meant. Similar to Shiki (and other widely used libraries) they still provide an easy to use and easy to bundle distributions. To keep the example with Shiki, see their bundle presets.
For recheck that would be the equivalent of keeping the current API exactly as is and consumers will notice no changes on their end.

// So this part does not change:
import { check, checkSync } from 'recheck';

// these will still work as is
const result = check();
const resultSync = checkSync();

// It just adds this part:
import { createCheck } from 'recheck/core';
import * as native from 'recheck/backend/native';

// this will be possible, on top of the 2 functions above
const customCheck = createCheck(native);
const resultsCustom = customCheck();

If the consumer does not care to optimize their build, there is 0 loss and it will work exactly as before.
But if the consumer does look into optimizing their build, they can either use the core functions to build their own check function, or use something like define which all bundlers implement.

By defining the process.env['RECHECK_BACKEND'] at build time, their bundler will be able to recognize the parts that are not used which in the case of using a binary recheck implementation would decrease the javascript code size from 3 mb to 4 kb (!!!), that is almost a 1000x decrease in size, just by bothering to add a line of configuration to their bundler.

I have already tried this with my prototype, and to me that makes it worth it to use the library. As having to bundle 4kb of JS + a native executable is not that high of a cost when you also get native executable performance.

Also be aware that the JS runtime is being forced to load a 3 mb javascript file, and having to parse and compile it, even if the inline webworker and pure scalajs is not used. require calls are (dynamic) function calls, and thus are limited in how bundlers and runtimes can optimize it. But import statements are static, thus allow for static analysis which is why tree-shaking (also know as dead code elimination) is so incredibly efficient when implemented properly.

---

To show some examples from the source code, and the issues they introduce:

the entry point of the package is written in cjs format. This already blocks all bundlers from optimizing the rest of the library. This is where the addition of exports to the package.json would already help, without changing anything for the consumer.

// index.js entry point in package.json#main
const lib = require("./lib/main");

exports.check = lib.check;
exports.checkSync = lib.checkSync;

The main.js is where it checks for which backend should be used.

// main.js
// RECHECK_BACKEND is a function returning 'process.env["RECHECK_BACKEND"] || "auto"'
const backend = env.RECHECK_BACKEND();
switch (backed) {
  case 'auto':   // try to detect the best case for the caller
  case 'java':   // use java
  case 'native': // use native
  case 'worker': // use worker
  case 'pure':   // use pure
}

This is very useful as a consumer, but a bundler/runtime cannot optimize this.
Imagine it looking more like this:

switch (process.env["RECHECK_BACKEND"]) {
  case 'auto':   // try to detect the best case for the caller
  case 'java':   // use java
  case 'native': // use native
  case 'worker': // use worker
  case 'pure':   // use pure
}

This way, if I configure my bundler to define RECHECK_BACKEND as native, the bundler will see this:

switch ('native') {
  case 'auto':   { /* try to detect the best case for the caller */ }
  case 'java':   { /* use java   */ }
  case 'native': { /* use native */ }
  case 'worker': { /* use worker */ }
  case 'pure':   { /* use pure   */ }
}

Which gets optimized to:

{ /* use native */ }

So any imports that are only used in the other blocks, will be dropped, the code in those imports will never be loaded/bundled which is what enables the size decrease from 3mb to 4kb.

---

I understand your concerns of maintability, if you think this is to much to maintain I can think of an alternative. Since the .jar and executables are separate packages, I could author and maintain a separate package, which uses those packages as optional dependencies just as recheck does, but in a way that is ESM compatible and tree-shakable. In that case I would only ask for a few additions to the package.json files so that bundlers and runtimes can resolve properly in ESM environments, and if possible to also distribute the pure build as a separate package. Worst case I can use a git subrepo to build the scala to js and distribute it as a package myself.

Let me know if you are open for any PR's, and ofcourse as I intend to submit PR's I also intend to fully document the changes and usage.

[makenowjust]

I appreciate your explanation. I understand your idea, and it sounds good. I welcome your PR. Thank you.

[kevinramharak]

@makenowjust This is the experimental prototype I wrote: main...kevinramharak:recheck:feature/provide-esm-build

Note that this is not what I intend to create a PR for. I ended up doing a rewrite as I went through the source line-by-line to understand what it actually does and how everything works together. Additionally I did this to figure out how to setup the ESM tree shaking part correctly. I intend to use this to suggest a smaller set of changes.

Below the recommended changes, split up into parts as I'm hoping to write some smaller PR's for each suggestion.
Let me know what you think and provide suggestions on how to minimize the long-term maintenance overhead.

Recommendations

Main issues:

• testing and mocking are to tightly coupled to the source code
• the webworker files output, inline and external
• use process.env['VAR'] directly, instead of behind an env.VAR() function call
• make sure functions are 'pure', to make allow tree shaking / dead code elimination to work

Testing and mocking

Currently the main.ts has a __mock__ variable, I can imagine why it was setup that way, but ESM support for testing has come a long way. I would suggest using vitest (ESM support) or experimental support in jest.

webworker files, inline and external

This one was really hard to figure out. I like the use of esbuild and its injection feature, but it is really hard to follow both for a human and a bundler. Initially my rewrite copied the approach, but I ended up taking a different approach that is typesafe and easier to follow:

• write the worker specific code in files ending with .worker.ts
• add these to esbuild as separate entry points, thus creating separate output files for them
• write a basic raw-loader plugin (or any custom loader plugin), that allows the createInlineWorker to be implemented with an import like import webWorkerSourceCode from "./web.worker.ts?raw";.

This will avoid the injection part, and enable the webworker code to be used be used as regular imports:

• shared parts
• thread worker
• web worker
• synkit worker
• example of how an createInlineWorker would work

Additionally this makes it very easy to distribute the worker as a separate file, which is not required, but a separate feature request I will create an issue for.

I have not tested the inline part, I realize I might be missing a compilation step there, but since its all implemented in the loader, its easy to change.

process.env['VAR']

Currently the environment variables are behind function calls in env.VAR(), to enable optimisation, these cannot be behind a function call. This is a minor change, but technically breaking if someone is dynamically changing the environment variables to use different backends in between check calls.

'pure' functions

Some functions might need a bit of restructuring and be made more 'pure' to allow bundlers to figure out what imports/variables can be dropped from a build. For example, when RECHECK_BIN or RECHECK_JAR is used, the used code path should not reference any imports or variables related to the scalajs implementation, or else that code cannot be dropped safely.