Skip to content

Commit f309811

Browse files
rekmarksclaude
andauthored
refactor(kernel-utils): replace vat bundler NODE_ENV define with a plugin (#967)
## Summary Moves `process.env.NODE_ENV` injection out of the `bundleVat` build config and into a dedicated, auto-detecting Rolldown transform plugin, resolving #812. Previously `bundleVat` configured a Vite `define` that textually substituted `process.env.NODE_ENV` → `"production"` on every build. It was a workaround for libraries (e.g. immer) that branch on `process.env.NODE_ENV`, but baking it into the build config coupled bundling to an env-shimming concern. The new `replaceNodeEnvPlugin` makes that logic a first-class, unit-tested plugin instead. ## Changes - **New `replaceNodeEnvPlugin`** (`packages/kernel-utils/src/vite-plugins/replace-node-env-plugin.ts`) — a Rolldown `transform` plugin that inlines `process.env.NODE_ENV` as a string literal in any module referencing it, and no-ops otherwise. Mirrors the sibling `removeDynamicImportsPlugin`. - **Configurable value** — the plugin accepts an options bag `{ value }` (type `ReplaceNodeEnvPluginOptions`), defaulting to `'production'`, so callers can inline a different `process.env.NODE_ENV` value. - **`bundle-vat.ts`** — removed the `define` block (and its TODO) and added `replaceNodeEnvPlugin()` to the `rolldownOptions.plugins` array (uses the `'production'` default). - **`index.ts`** — exports the new plugin and its options type alongside `removeDynamicImportsPlugin`. - **Tests** — `replace-node-env-plugin.test.ts` covers positive/negative cases, the quoted-literal assertion, the word-boundary near-miss (`NODE_ENVIRONMENT`), and a configured non-default value. ## Behavior For the default value, output is identical to the old `define` for any bundle referencing `process.env.NODE_ENV` — the logic is just relocated to a named, tested plugin. ## Verification - `yarn workspace @metamask/kernel-utils build` — passes. - `yarn workspace @metamask/kernel-utils test:dev:quiet` — passes (new + existing plugin/index tests). - End-to-end: bundling a fixture that reads `process.env.NODE_ENV` produced `var isProd = true; var rawMode = "production";` — fully inlined and constant-folded, no residual `process.env.NODE_ENV` reference. - `yarn lint` — clean. Closes #812 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Build-time bundling refactor with equivalent default behavior; limited to kernel-utils vat bundling and covered by new tests. > > **Overview** > **`bundleVat`** no longer sets a global Vite `define` for `process.env.NODE_ENV`. That injection is handled by a new Rolldown **`replaceNodeEnvPlugin`**, wired into the vat bundle pipeline alongside the existing dynamic-import stripper. > > The plugin only transforms modules that reference `process.env.NODE_ENV`, replacing them with a JSON-stringified literal (default **`"production"`**, overridable via `{ value }`). That keeps vat bundles safe for deps like immer when there is no runtime `process` global, without coupling every build to a blanket define. > > **`replaceNodeEnvPlugin`** and **`ReplaceNodeEnvPluginOptions`** are exported from **`./vite-plugins`**, with unit tests covering replacements, no-ops, word boundaries, and custom values. CHANGELOG documents the addition. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 8f73140. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9ff2ab7 commit f309811

5 files changed

Lines changed: 127 additions & 6 deletions

File tree

packages/kernel-utils/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- Add an optional `required` field to `MethodSchema` (mirroring `required` on object `JsonSchema`) naming which arguments are required, and a `{ required }` option on `methodArgsToStruct` that validates unlisted arguments as optional, so a method's argument schema can faithfully represent the optional trailing arguments its guard already allows ([#958](https://github.com/MetaMask/ocap-kernel/pull/958))
1414
- Add `getLibp2pRelayHome()` to the `./nodejs` exports, returning the libp2p relay's bookkeeping directory (default `~/.libp2p-relay`, overridable via `$LIBP2P_RELAY_HOME`) — kept separate from `$OCAP_HOME` so one relay can serve daemons with different OCAP_HOMEs ([#952](https://github.com/MetaMask/ocap-kernel/pull/952))
1515
- `startRelay()` accepts an optional `publicIp` that is fed to libp2p's `appendAnnounce`, so a relay running on a NAT-backed host can announce its publicly-reachable IPv4 alongside its bound NIC addresses ([#952](https://github.com/MetaMask/ocap-kernel/pull/952))
16+
- Add `replaceNodeEnvPlugin` to the `./vite-plugins` exports, a Rolldown transform plugin that inlines `process.env.NODE_ENV` as a string literal (configurable via `{ value }`, defaulting to `'production'`) in any module referencing it — used by `bundleVat` in place of a build-config `define`, so vats that pull in libraries like immer resolve the reference at bundle time ([#967](https://github.com/MetaMask/ocap-kernel/pull/967))
1617

1718
## [0.5.0]
1819

packages/kernel-utils/src/vite-plugins/bundle-vat.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { build } from 'vite';
77

88
import type { VatBundle } from '../vat-bundle.ts';
99
import { exportMetadataPlugin } from './export-metadata-plugin.ts';
10+
import { replaceNodeEnvPlugin } from './replace-node-env-plugin.ts';
1011
import { stripCommentsPlugin } from './strip-comments-plugin.ts';
1112

1213
export type { VatBundle } from '../vat-bundle.ts';
@@ -88,12 +89,6 @@ export async function bundleVat(sourcePath: string): Promise<VatBundle> {
8889
result = await build({
8990
configFile: false,
9091
logLevel: 'silent',
91-
// TODO: Remove this define block and add a process shim to VatSupervisor
92-
// workerEndowments instead. This injects into ALL bundles but is only needed
93-
// for libraries like immer that check process.env.NODE_ENV.
94-
define: {
95-
'process.env.NODE_ENV': JSON.stringify('production'),
96-
},
9792
build: {
9893
write: false,
9994
minify: false,
@@ -109,6 +104,7 @@ export async function bundleVat(sourcePath: string): Promise<VatBundle> {
109104
},
110105
plugins: [
111106
removeDynamicImportsPlugin(),
107+
replaceNodeEnvPlugin(),
112108
stripCommentsPlugin(),
113109
metadataPlugin,
114110
],

packages/kernel-utils/src/vite-plugins/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { bundleVat } from './bundle-vat.ts';
44

55
export { bundleVat, removeDynamicImportsPlugin } from './bundle-vat.ts';
66
export type { VatBundle } from './bundle-vat.ts';
7+
export { replaceNodeEnvPlugin } from './replace-node-env-plugin.ts';
8+
export type { ReplaceNodeEnvPluginOptions } from './replace-node-env-plugin.ts';
79

810
type VatEntry = {
911
/** Absolute path to the vat source file */
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { describe, it, expect } from 'vitest';
2+
3+
import { replaceNodeEnvPlugin } from './replace-node-env-plugin.ts';
4+
5+
type TransformFn = (
6+
code: string,
7+
id: string,
8+
) => { code: string; map: null } | null;
9+
10+
describe('replaceNodeEnvPlugin', () => {
11+
const plugin = replaceNodeEnvPlugin();
12+
const transform = plugin.transform as TransformFn;
13+
14+
it.each([
15+
['bare reference', `process.env.NODE_ENV`, `"production"`],
16+
[
17+
'reference in a conditional',
18+
`if (process.env.NODE_ENV !== 'production') { warn(); }`,
19+
`if ("production" !== 'production') { warn(); }`,
20+
],
21+
[
22+
'multiple references in one file',
23+
`const a = process.env.NODE_ENV;\nconst b = process.env.NODE_ENV;`,
24+
`const a = "production";\nconst b = "production";`,
25+
],
26+
[
27+
'reference surrounded by other code',
28+
`const x = 1;\nexport const mode = process.env.NODE_ENV;\nconst y = 2;`,
29+
`const x = 1;\nexport const mode = "production";\nconst y = 2;`,
30+
],
31+
])(
32+
'replaces %s with the "production" string literal',
33+
(_name, code, expected) => {
34+
const result = transform(code, 'test.ts');
35+
expect(result).toStrictEqual({ code: expected, map: null });
36+
},
37+
);
38+
39+
it('injects a quoted string literal, not the bare word', () => {
40+
const result = transform(`const mode = process.env.NODE_ENV;`, 'test.ts');
41+
expect(result?.code).toContain(`"production"`);
42+
expect(result?.code).not.toContain(`process.env.NODE_ENV`);
43+
expect(result?.code).not.toContain(`= production;`);
44+
});
45+
46+
it.each([
47+
['no process reference', 'const x = 1;'],
48+
['unrelated process.env key', `const port = process.env.PORT;`],
49+
['empty code', ''],
50+
])('returns null for %s', (_name, code) => {
51+
expect(transform(code, 'test.ts')).toBeNull();
52+
});
53+
54+
it('returns null when the substring matches but the word boundary does not', () => {
55+
const code = `const x = process.env.NODE_ENVIRONMENT;`;
56+
expect(transform(code, 'test.ts')).toBeNull();
57+
});
58+
59+
it('inlines a configured value instead of the default', () => {
60+
const devTransform = replaceNodeEnvPlugin({ value: 'development' })
61+
.transform as TransformFn;
62+
const result = devTransform(
63+
`const mode = process.env.NODE_ENV;`,
64+
'test.ts',
65+
);
66+
expect(result).toStrictEqual({
67+
code: `const mode = "development";`,
68+
map: null,
69+
});
70+
});
71+
});
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import type { Plugin as RolldownPlugin } from 'rolldown';
2+
3+
export type ReplaceNodeEnvPluginOptions = {
4+
/**
5+
* The value to inline for `process.env.NODE_ENV`. Defaults to `'production'`.
6+
*/
7+
value?: string;
8+
};
9+
10+
/**
11+
* A Rolldown plugin that inlines `process.env.NODE_ENV` as a string literal in
12+
* any module that references it.
13+
*
14+
* This replaces the former build-config `define` (issue #812). Libraries such
15+
* as immer branch on `process.env.NODE_ENV`, and vats have no `process` global
16+
* at runtime, so the reference must be resolved at bundle time. The plugin
17+
* auto-detects the need: modules without the reference are left untouched.
18+
*
19+
* This is a textual replacement (the same class of approach as the sibling
20+
* {@link removeDynamicImportsPlugin}) and handles the dotted
21+
* `process.env.NODE_ENV` form, consistent with the `define` it replaces.
22+
*
23+
* @param options - Plugin options.
24+
* @param options.value - The value to inline for `process.env.NODE_ENV`.
25+
* Defaults to `'production'`.
26+
* @returns A Rolldown plugin.
27+
*/
28+
export function replaceNodeEnvPlugin({
29+
value = 'production',
30+
}: ReplaceNodeEnvPluginOptions = {}): RolldownPlugin {
31+
const replacement = JSON.stringify(value);
32+
return {
33+
name: 'ocap-kernel:replace-node-env',
34+
transform(code) {
35+
if (!code.includes('process.env.NODE_ENV')) {
36+
return null;
37+
}
38+
39+
const transformed = code.replace(
40+
/\bprocess\.env\.NODE_ENV\b/gu,
41+
replacement,
42+
);
43+
44+
if (transformed === code) {
45+
return null;
46+
}
47+
48+
return { code: transformed, map: null };
49+
},
50+
};
51+
}

0 commit comments

Comments
 (0)