Skip to content

feat(bundler): define BundlerConfig public API #5882

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
killagu wants to merge 1 commit into
base: split/10-bundler-generate-manifest
Choose a base branch
from
+55 −1
Open

feat(bundler): define BundlerConfig public API #5882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion tools/egg-bundler/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,46 @@
export {};
export { ExternalsResolver, type ExternalsConfig, type ExternalsResolverOptions } from './lib/ExternalsResolver.ts';
export { ManifestLoader, type ManifestLoaderOptions } from './lib/ManifestLoader.ts';
Comment on lines +1 to +2
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In TypeScript ESM, import and export paths should use the .js extension (or the extension that will exist at runtime) rather than .ts. Using .ts extensions in import paths is non-standard and will cause errors in Node.js and most TypeScript configurations unless specific flags like allowImportingTsExtensions are used. For standard ESM compatibility, it is recommended to use .js extensions in the source code.

Suggested change
export { ExternalsResolver, type ExternalsConfig, type ExternalsResolverOptions } from './lib/ExternalsResolver.ts';
export { ManifestLoader, type ManifestLoaderOptions } from './lib/ManifestLoader.ts';
export { ExternalsResolver, type ExternalsConfig, type ExternalsResolverOptions } from './lib/ExternalsResolver.js';
export { ManifestLoader, type ManifestLoaderOptions } from './lib/ManifestLoader.js';


export interface BundlerExternalsConfig {
/** Package names to always mark as external, in addition to auto-detected ones. */
readonly force?: readonly string[];
/** Package names to never mark as external (force inline), overriding auto-detection. */
readonly inline?: readonly string[];
}

export interface BundlerPackConfig {
/** Override for the monorepo workspace root. Defaults to auto-detection. */
readonly rootPath?: string;
}

export interface BundlerConfig {
/** Application root directory. Required. */
readonly baseDir: string;
/** Output directory for the bundled artifact. Required. */
readonly outputDir: string;
/** Path to manifest.json. Defaults to `<baseDir>/.egg/manifest.json`. */
readonly manifestPath?: string;
/** Framework name or absolute path. Defaults to `'egg'`. */
readonly framework?: string;
/** Build mode. Defaults to `'production'`. */
readonly mode?: 'production' | 'development';
/** External package overrides. */
readonly externals?: BundlerExternalsConfig;
/** @utoo/pack tuning. */
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The reference to @utoo/pack appears to be a typo for @eggjs/pack. Using the consistent @eggjs organization prefix is recommended for clarity and correctness, unless @utoo/pack is a specific internal dependency intended for this configuration.

Suggested change
/** @utoo/pack tuning. */
/** @eggjs/pack tuning. */

readonly pack?: BundlerPackConfig;
/** Enable tegg decoratedFile collection. Defaults to `true`. */
readonly tegg?: boolean;
}
Comment on lines +16 to +33
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc on BundlerConfig describes runtime defaults/behavior, but bundle() currently always throws, so those defaults are not actually applied yet. To avoid misleading API docs, either (a) add a doc comment on bundle() explicitly stating it is a placeholder that always throws (and that config defaults are provisional), or (b) adjust the config field docs to indicate the defaults are planned (until implementation lands).

Copilot uses AI. Check for mistakes.

export interface BundleResult {
/** Absolute path to the output directory. */
readonly outputDir: string;
/** All artifact files (absolute paths), sorted. */
readonly files: readonly string[];
/** Absolute path to the normalized bundled manifest. */
readonly manifestPath: string;
}

Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JSDoc on BundlerConfig describes runtime defaults/behavior, but bundle() currently always throws, so those defaults are not actually applied yet. To avoid misleading API docs, either (a) add a doc comment on bundle() explicitly stating it is a placeholder that always throws (and that config defaults are provisional), or (b) adjust the config field docs to indicate the defaults are planned (until implementation lands).

Suggested change
/**
* Placeholder API for the future bundling implementation.
*
* This function is not implemented yet and always throws.
* The defaults described on {@link BundlerConfig} are planned behavior and
* are not currently applied at runtime.
*/

Copilot uses AI. Check for mistakes.
export async function bundle(_config: BundlerConfig): Promise<BundleResult> {
throw new Error('@eggjs/egg-bundler: bundle() is not implemented yet');
}
9 changes: 9 additions & 0 deletions tools/egg-bundler/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { describe, expect, it } from 'vitest';

import { bundle } from '../src/index.ts';
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with ESM standards, it is recommended to use the .js extension in import paths, even when referencing .ts files. While Vitest handles .ts extensions, using .js is the standard approach for TypeScript ESM to ensure compatibility across different environments.

Suggested change
import { bundle } from '../src/index.ts';
import { bundle } from '../src/index.js';


describe('@eggjs/egg-bundler', () => {
Comment on lines +1 to +5
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This smoke test verifies the placeholder error, but it doesn’t assert the new public API exports beyond bundle() (e.g., that bundle’s signature stays (_config: BundlerConfig) => Promise<BundleResult>). Consider adding a type-level assertion using Vitest’s expectTypeOf(...) (or a tsd-style test) to lock in the exported types/signature as part of defining the public API.

Suggested change
import { describe, expect, it } from 'vitest';
import { bundle } from '../src/index.ts';
describe('@eggjs/egg-bundler', () => {
import { describe, expect, expectTypeOf, it } from 'vitest';
import { bundle, type BundleResult, type BundlerConfig } from '../src/index.ts';
describe('@eggjs/egg-bundler', () => {
it('exports bundle() with the expected public API signature', () => {
expectTypeOf(bundle).toEqualTypeOf<(_config: BundlerConfig) => Promise<BundleResult>>();
});

Copilot uses AI. Check for mistakes.
it('bundle() is a placeholder that throws until implemented', async () => {
await expect(bundle({ baseDir: '/tmp', outputDir: '/tmp/out' })).rejects.toThrow(/not implemented/);
Comment on lines +1 to +7
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test hard-codes /tmp paths, which is not cross-platform (fails on Windows) and may also cause collisions across parallel test runs. Use os.tmpdir() + fs.mkdtemp() (or a test temp-dir helper) to create a unique temporary directory, and build baseDir/outputDir with path.join().

Suggested change
import { describe, expect, it } from 'vitest';
import { bundle } from '../src/index.ts';
describe('@eggjs/egg-bundler', () => {
it('bundle() is a placeholder that throws until implemented', async () => {
await expect(bundle({ baseDir: '/tmp', outputDir: '/tmp/out' })).rejects.toThrow(/not implemented/);
import { mkdtemp } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import * as path from 'node:path';
import { describe, expect, it } from 'vitest';
import { bundle } from '../src/index.ts';
describe('@eggjs/egg-bundler', () => {
it('bundle() is a placeholder that throws until implemented', async () => {
const baseDir = await mkdtemp(path.join(tmpdir(), 'egg-bundler-'));
const outputDir = path.join(baseDir, 'out');
await expect(bundle({ baseDir, outputDir })).rejects.toThrow(/not implemented/);

Copilot uses AI. Check for mistakes.
});
});