egoist / egoist/tsup

Using shims and codesplitting together has subtle issues.

Open
#958 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
11.3k
Forks
275
PR merge metrics
No merged PRs in 30d

Description

All of these edge cases come into play when the shims end up in a different chunk than they're used in.

## CJS shim edge cases
When outputting a ESM bundle with shims enabled:
* `__filename` will never be correct as the `import.meta.url` will be the chunk it's exported from, not the one it's used in.
* `__dirname` will be incorrect if the chunk containing the shims is in a different directory than the chunk importing the shims.
```
dist
├── a
│ └── index.js // <- __filename will be /dist/chunk-with-shims.js, not /dist/a/index.js
├── b
│ └── index.js // <- if the __dirname shims are imported in this chunk, it will be /dist, not /dist/b
├── c
│ └── index.js
├── chunk-with-shims.js
└── ...
```

## Edge cases when using ESM shims

For the ESM shims, `import.meta.url` will never be correct if the shims are in a chunk.
```
dist
├── a
│ └── index.js
├── b
│ └── index.js
├── c
│ └── index.js
├── chunk-with-shims.js <- the importMetaUrl export uses __filename, which will always be dist/chunk-with-shims.js
└── ...
```

```js
var _shimChunk = require('../chunk-with-shims.js');
var _fs = require('fs');
var _fs2 = _interopRequireDefault(_fs);

// This is adapted from https://nodejs.org/api/esm.html#importmetaurl
// _shimChunk.importMetaUrl will be the __filename of `../chunk-with-shims.js`, not this file
const buffer = _fs2.default.readFileSync(new URL('./data.proto', _shimChunk.importMetaUrl));
```

One impact of this, is that it breaks checking whether a module is `main`. https://exploringjs.com/nodejs-shell-scripting/ch_nodejs-path.html#detecting-if-module-is-main

```js
import * as url from 'node:url';

if (import.meta.url.startsWith('file:')) { // (A)
// import.meta.url will be wrong if the shim ends up in a chunk, and this
// will never run when called from a shell.
const modulePath = url.fileURLToPath(import.meta.url);
if (process.argv[1] === modulePath) { // (B)
// Main ESM module
}
}
```

## How to fix it

The ESM/CJS shims should be injected into each chunk that requires them, rather than using the `inject` feature of `esbuild` as it's currently done. https://github.com/egoist/tsup/blob/v7.1.0/src/esbuild/index.ts#L220

This could be done with an ESBuild plugin, which modifies the output chunks after the build, or as a tsup plugin.

This is a pretty hacky plugin I whipped up to replace the `tsup` shims when building ESM bundles. Having poked around the code base, (and seeing how tsup plugins work), this likely breaks sourcemaps, but they aren't turned on in our project.

```ts
import { Options } from "tsup";
import * as path from "path";
import * as fs from "fs";

// https://stackoverflow.com/a/51399781
type ArrayElement =
ArrayType extends readonly (infer ElementType)[] ? ElementType : never;

type EsbuildPlugin = ArrayElement>;

const CjsShimPlugin: EsbuildPlugin = {
name: "cjs-shim-plugin",
setup(build) {
build.onEnd((result) => {
const filesNeedingShims =
result.outputFiles?.filter(
(file) => file.text.match(/__dirname|__filename/) !== null
) || [];

filesNeedingShims.map((file) => {
const dirnameShim = `
import { fileURLToPath as fileURLToPathShimImport } from "node:url";
import { dirname as dirnamePathShimImport } from "node:path";
const __filename = fileURLToPathShimImport(import.meta.url);
const __dirname = dirnamePathShimImport(__filename);
`;

const shimBuffer = Buffer.from(dirnameShim);
file.contents = Buffer.concat([shimBuffer, file.contents]);
});
});
},
};

export default CjsShimPlugin;

```

Contributor guide

Open the contributing guide

Research direction

Start in src/esbuild/index.ts around the injection code linked at v7.1.0 line 220, then review the issue's proposed esbuild plugin and its onEnd handling of outputFiles. Done means ESM and CJS shims are present in each chunk that needs them and preserve the correct chunk-relative paths, including the main-module case.

Written by the indexing model from the issue text.

Assessment

Tech stack
nodejs, typescript
Domain
build-system, tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.