Skip to content

Commit 8e6909e

Browse files
committed
feat(bundler): implement policy for external import specifiers in bundling process
- Introduced a new policy to handle external import specifiers, ensuring they remain unchanged in bundled output to maintain compatibility across Deno, Node.js, and Bun. - Updated the Rolldown backend and import map resolver to support explicit external marking and prevent rewriting of bare specifiers. - Enhanced bundler configuration to allow for explicit externals, improving module instance sharing and reducing potential runtime issues.
1 parent 6a48d21 commit 8e6909e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+337
-66
lines changed
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
# Bundler External Import Specifiers Policy
2+
3+
## Status
4+
5+
Accepted
6+
7+
## Context and Problem Statement
8+
9+
When bundling JavaScript/TypeScript code, some imports are marked as "external"
10+
(not inlined into the bundle). The bundler must decide what import specifier to
11+
use in the output for these external imports.
12+
13+
```typescript
14+
// Source file (with import map resolving @foo/bar to jsr:@foo/bar@^1.0.0)
15+
import { something } from "@foo/bar";
16+
17+
// Bundled output - what specifier should appear?
18+
import { something } from "???";
19+
```
20+
21+
The bundler has access to the project's import map and can resolve bare
22+
specifiers to their full forms (e.g., `jsr:@foo/bar@^1.0.0`). The question is:
23+
**should it rewrite the specifier or keep it as-is?**
24+
25+
## Decision Drivers
26+
27+
- **Cross-runtime compatibility**: Output must work in Deno, Node.js, and Bun
28+
- **Standard resolution**: Use resolution mechanisms all runtimes understand
29+
- **No runtime dependencies on import maps**: Bundled code should not require
30+
import maps to be present at runtime
31+
- **Simplicity**: Avoid unnecessary transformations
32+
33+
## Decision Outcome
34+
35+
**NEVER rewrite external import specifiers to protocol-specific forms.**
36+
37+
Keep bare specifiers as-is. They resolve from `node_modules` at runtime, which
38+
works in all JavaScript runtimes.
39+
40+
### The Rule
41+
42+
| Source Specifier | Bundled Output | Correct? |
43+
| ---------------- | --------------------- | -------- |
44+
| `@foo/bar` | `@foo/bar` | ✅ YES |
45+
| `@foo/bar` | `jsr:@foo/bar@^1.0.0` | ❌ NO |
46+
| `@foo/bar` | `npm:@foo/bar` | ❌ NO |
47+
| `lodash` | `lodash` | ✅ YES |
48+
| `lodash` | `npm:lodash@^4.0.0` | ❌ NO |
49+
50+
### Why Protocol Specifiers Break
51+
52+
```typescript
53+
// This ONLY works in Deno
54+
import { x } from "jsr:@foo/bar@^1.0.0";
55+
56+
// This ONLY works in Deno
57+
import { x } from "npm:lodash@^4.0.0";
58+
59+
// This works EVERYWHERE (Deno, Node.js, Bun)
60+
import { x } from "@foo/bar";
61+
import { y } from "lodash";
62+
```
63+
64+
- `jsr:` protocol - Deno-only, not understood by Node.js or Bun
65+
- `npm:` protocol - Deno-only, not understood by Node.js or Bun
66+
- Bare specifiers - Standard, resolved from `node_modules` by all runtimes
67+
68+
### Consequences
69+
70+
#### Good
71+
72+
- Bundled code works in Deno, Node.js, and Bun
73+
- Uses standard `node_modules` resolution
74+
- No special runtime configuration needed
75+
- Simpler bundler logic (no specifier rewriting)
76+
77+
#### Bad
78+
79+
- Requires dependencies to be installed in `node_modules`
80+
- Bare specifiers are less explicit about versions
81+
82+
## Implementation
83+
84+
### In Rolldown Backend (`@eser/bundler`)
85+
86+
Convert external package names to regex patterns for prefix matching:
87+
88+
```typescript
89+
// In rolldown.ts - bundle() and watch()
90+
const externalPatterns = config.external?.map((pkg) => {
91+
const escaped = pkg.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
92+
return new RegExp(`^${escaped}(\\/.*)?$`);
93+
});
94+
95+
const bundle = await rolldown.rolldown({
96+
input: config.entrypoints,
97+
external: externalPatterns, // Regex patterns, not strings
98+
plugins,
99+
});
100+
```
101+
102+
This ensures `"@eser/laroux-server"` matches both:
103+
104+
- `@eser/laroux-server` (exact)
105+
- `@eser/laroux-server/action-registry` (subpath)
106+
107+
### In Import Map Resolver Plugin
108+
109+
Handle `{ external: true }` without a path in the resolveId hook:
110+
111+
```typescript
112+
// In rolldown.ts - adaptPlugins()
113+
if (result?.path !== undefined || result?.external === true) {
114+
return {
115+
id: result.path ?? source, // Keep original specifier if no path
116+
external: result.external,
117+
};
118+
}
119+
```
120+
121+
Use `autoMarkExternal` option to control automatic external marking:
122+
123+
```typescript
124+
// For client bundling (default): autoMarkExternal = true
125+
// For server bundling: autoMarkExternal = false (use explicit externals)
126+
createImportMapResolverPlugin({
127+
projectRoot,
128+
browserShims: { jsr: {}, nodeBuiltins: {} },
129+
autoMarkExternal: false, // Don't auto-mark npm/jsr as external
130+
});
131+
```
132+
133+
### In Bundler Configuration
134+
135+
When specifying externals, use the package name (not subpaths):
136+
137+
```typescript
138+
// CORRECT - package name covers all subpath imports via regex
139+
externals: ["@eser/laroux-server", "react", "lodash"];
140+
141+
// The output will have:
142+
// import { registerAction } from "@eser/laroux-server/action-registry";
143+
// import { something } from "@eser/laroux-server/other";
144+
// import React from "react";
145+
// import _ from "lodash";
146+
```
147+
148+
## How Bare Specifiers Resolve at Runtime
149+
150+
All runtimes resolve bare specifiers by looking in `node_modules`:
151+
152+
```
153+
project/
154+
├── node_modules/
155+
│ ├── @eser/
156+
│ │ └── laroux-server/ ← @eser/laroux-server resolves here
157+
│ ├── react/ ← react resolves here
158+
│ └── lodash/ ← lodash resolves here
159+
├── dist/
160+
│ └── bundle.js ← bundled code with bare imports
161+
├── deno.json ← nodeModulesDir: "auto" for Deno
162+
└── package.json ← dependencies for npm/bun
163+
```
164+
165+
**Deno**: With `"nodeModulesDir": "auto"` in `deno.json`, Deno creates
166+
`node_modules` from JSR/npm dependencies and uses it for resolution.
167+
168+
**Node.js**: Standard `node_modules` resolution per Node.js module algorithm.
169+
170+
**Bun**: Standard `node_modules` resolution, compatible with Node.js.
171+
172+
## Common Mistakes
173+
174+
### Mistake 1: Rewriting to JSR specifiers
175+
176+
```typescript
177+
// Source
178+
import { foo } from "@eser/something";
179+
180+
// WRONG output - breaks Node.js and Bun
181+
import { foo } from "jsr:@eser/something@^4.0.0";
182+
```
183+
184+
### Mistake 2: Rewriting to npm specifiers
185+
186+
```typescript
187+
// Source
188+
import { foo } from "lodash";
189+
190+
// WRONG output - breaks Node.js and Bun
191+
import { foo } from "npm:lodash@^4.0.0";
192+
```
193+
194+
### Mistake 3: Using resolved paths for externals
195+
196+
```typescript
197+
// In resolver plugin
198+
const resolved = importMap.resolve(specifier); // "jsr:@foo/bar@^1.0.0"
199+
return { path: resolved, external: true }; // ❌ WRONG
200+
201+
// Correct
202+
return { external: true }; // ✅ Keeps original bare specifier
203+
```
204+
205+
## Applies To
206+
207+
This policy applies to ALL bundling operations in the stack:
208+
209+
- **Client bundling** (`bundle()` in `domain/bundler.ts`)
210+
- **Server bundling** (`bundleServerComponents()` in `domain/bundler.ts`)
211+
- **Any future bundling** operations
212+
213+
## Related Files
214+
215+
- `pkg/@eser/bundler/backends/rolldown.ts` - Rolldown backend (external regex
216+
patterns)
217+
- `pkg/@eser/laroux-bundler/import-map-resolver-plugin.ts` - Import resolution
218+
(autoMarkExternal)
219+
- `pkg/@eser/laroux-bundler/system.ts` - Build system (server bundling config)
220+
- `pkg/@eser/laroux-bundler/domain/bundler.ts` - Bundler functions
221+
222+
## Links
223+
224+
- [Deno nodeModulesDir](https://docs.deno.com/runtime/manual/node/npm_specifiers)
225+
- [Node.js Module Resolution](https://nodejs.org/api/modules.html)
226+
- [Bun Module Resolution](https://bun.sh/docs/runtime/modules)

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eser/stack",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"private": true,
55
"type": "module",
66
"workspaces": [

pkg/@cool/cli/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@cool/cli",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"type": "module",
55
"exports": {
66
".": "./mod.ts"

pkg/@cool/lime/deno.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@cool/lime",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"exports": {
55
".": "./mod.ts"
66
}

pkg/@cool/lime/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@cool/lime",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"type": "module",
55
"exports": {
66
".": "./mod.ts"

pkg/@eser/app-runtime/deno.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eser/app-runtime",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"exports": {
55
".": "./mod.ts"
66
},

pkg/@eser/app-runtime/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eser/app-runtime",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"type": "module",
55
"exports": {
66
".": "./mod.ts"

pkg/@eser/bundler/backends/rolldown.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,9 +370,12 @@ export class RolldownBackend implements Bundler {
370370
namespace: resolver.options.namespace ?? "file",
371371
kind: "import-statement",
372372
});
373-
if (result?.path !== undefined) {
373+
// Handle both path resolution and external marking
374+
// External without path keeps original specifier (bare import from node_modules)
375+
// See ADR: 0002-bundler-external-import-specifiers.md
376+
if (result?.path !== undefined || result?.external === true) {
374377
return {
375-
id: result.path,
378+
id: result.path ?? source,
376379
external: result.external,
377380
};
378381
}

pkg/@eser/bundler/deno.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eser/bundler",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"exports": {
55
".": "./mod.ts",
66
"./types": "./types.ts",

pkg/@eser/bundler/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eser/bundler",
3-
"version": "4.0.33",
3+
"version": "4.0.34",
44
"type": "module",
55
"exports": {
66
".": "./mod.ts"

0 commit comments

Comments
 (0)