fabian-hiller / valibot

The modular and type safe schema library for validating structural data 🤖
https://valibot.dev
MIT License
5.73k stars 173 forks source link

Avoid barrel files and create distinct files for exports #425

Open mxdvl opened 5 months ago

mxdvl commented 5 months ago

This library should consider whether exporting individual files for each of its constituent modules might improve performance and bundle size analysis.

Currently, it relies on “barrel files” which re-export all files, meaning the whole module graph must be built at all times. Marvin Hagemeister elaborates on why this should probably be avoided for performance reasons in “Speeding up the JavaScript ecosystem - The barrel file debacle”.

Analysing the bundle size with tools like tools like bundlephobia is very difficult, because it’s only via a bundler’s ability to tree shake the module that the small size is achieved. This also means that loading valibot from a CDN like esm.sh or jsdelivr will likely load the entire 11kB gzipped.

I beleive this could be achieved by simply disabling the bundle config in tsup. Worth noting that on my machine this leads to the DTS failing, akin to this known issue.

fabian-hiller commented 5 months ago

Thank you very much! I will read the article in the next few days and give feedback here.

lottamus commented 5 months ago

@mxdvl have you tried using the experiemental-dts option? I was able to build successfully with it enabled, though it seems pretty unstable (experimental).

import { defineConfig } from 'tsup';

export default defineConfig({
  entry: ['./src/**/!(*.test).ts'],
  clean: true,
  format: ['esm', 'cjs'],
  outDir: './dist',
  experimentalDts: true,
  bundle: false,
});

Alternatively, I found just emitting the dts alone after tsup works fine:

"build": "tsup && tsc --emitDeclarationOnly --declaration --noEmit false --outDir ./dist"

This setup also requires updating the package exports to include the deeply nested paths:

"exports": {
  ".": {
    "import": {
      "types": "./dist/index.d.ts",
      "default": "./dist/index.js"
    },
    "require": {
      "types": "./dist/index.d.ts",
      "default": "./dist/index.cjs"
    }
  },
  "./*": {
    "import": {
      "types": "./dist/*/index.d.ts",
      "default": "./dist/*/index.js"
    },
    "require": {
      "types": "./dist/*/index.d.ts",
      "default": "./dist/*/index.cjs"
    }
  }
},

And since disabling the tsup bundle options, I needed to remove the ".ts" extensions from imports (which turned out to be a ton of files 😅 ) and switch to "moduleResolution": "bundler" (seems counterintuitive but it removes the file extension requirements).

image

Finally, this allows me to do any of the following import styles:

import { object } from "valibot";
import { object } from "valibot/schemas";
import { object } from "valibot/schemas/object";

The output:

image image

Not sure if this was helpful or not, but I'm happy to push up my code for you to play with.


@fabian-hiller I noticed this when I saw an error message that includes the EMOJI_REGEX, as mentioned in this issue, which I wouldn't expect to be included in my app's bundle. I was investigating why the build wasn't tree-shaking only the functions I'm importing and came across this issue.

The nextjs team has been working on a solution to barrel files, however valibot is currently being bundled into a single file (not a barrel).

mxdvl commented 5 months ago

@lottamus – I think this is great work. I think that a tool like dts-buddy, when it comes out, would make the DevX experience a lot better, too.

And since disabling the tsup bundle options, I needed to remove the ".ts" extensions from imports (which turned out to be a ton of files 😅 ) and switch to "moduleResolution": "bundler" (seems counterintuitive but it removes the file extension requirements).

I think the esbuild outExtension option might do the trick, here?

Not sure if this was helpful or not, but I'm happy to push up my code for you to play with.

Please do! Even if just a branch on your fork.

Finally, this allows me to do any of the following import styles:

import { object } from "valibot";
import { object } from "valibot/schemas";
import { object } from "valibot/schemas/object";

I see little value in the second one "valibot/schemas"… is there any benefit to it?

I’m tempted to think that the main index file should looks something like:

import * as errors from './error/index.ts';
import * as methods from './methods/index.ts';
import * as regex from './regex.ts';
import * as schemas from './schemas/index.ts';
import * as storages from './storages/index.ts';
import * as transformations from './transformations/index.ts';
import * as types from './types/index.ts';
import * as utils from './utils/index.ts';
import * as validations from './validations/index.ts';

export const v = {
  ...errors,
  ...methods,
  ...regex,
  ...schemas,
  ...storages,
  ...transformations,
  ...types,
  ...utils,
  ...validations,
};

export default v;
fabian-hiller commented 5 months ago

I have to say that I don't really understand this issue yet. I don't see any downside to the barrel files when working on Valibot's source code. I also don't quite understand why it is an advantage not to bundle the library into a single file in the build step. It shouldn't affect tree shaking and according to Marvin's article the performance is better when importing Valibot from a single file. I don't think the CDN argument is quite right either, because if everything has to be imported individually, gzip compression can't be applied, which means that in the end probably even more bytes have to be downloaded. I would be happy to get feedback on this.

mxdvl commented 5 months ago

I don't see any downside to the barrel files when working on Valibot's source code.

I was only considering it from the built package’s perspective, not necessarily the source code.

I also don't quite understand why it is an advantage not to bundle the library into a single file in the build step. It shouldn't affect tree shaking and according to Marvin's article the performance is better when importing Valibot from a single file. I don't think the CDN argument is quite right either, because if everything has to be imported individually, gzip compression can't be applied, which means that in the end probably even more bytes have to be downloaded.

As the project is currently set up, it’s not possible to inspect the built artefact and understand how the module system works or which are its heavies parts, because everything is brought into a single file. While you are correct that tree-shaking can still happen, with explicit modules you would not even need to tree shake at all. I believe that being able to get granular information about the size of each module without having to go through a bundler would be beneficial, using tools such as https://github.com/preactjs/compressed-size-action, to ensure that there is no drift over time from your focus on small bundle size.


If you do not see the value of this proposal, feel free to close this issue as not planned.

fabian-hiller commented 5 months ago

Thanks for your response. This is interesting and I will look into it once the first v1 release candidates are ready. So the basic idea is to export each module as a submodule in addition to the global index file?

I will send you an email next week with some more ideas I plan to work on that could benefit your codebase.