Skip to content

Fix module resolution and Vite bundling for browser builds #498

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

Merged
hotlong merged 3 commits into from
Feb 3, 2026
Merged

Fix module resolution and Vite bundling for browser builds #498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6d90d4
Initial plan
Copilot Feb 3, 2026
6dc6e9a
Fix module resolution for example packages - add exports field
Copilot Feb 3, 2026
1e0c9b0
Fix Vite browser bundling - make Node.js module imports conditional
Copilot Feb 3, 2026
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
6 changes: 6 additions & 0 deletions examples/app-crm/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
"version": "0.9.0",
"description": "Example CRM implementation using ObjectStack Protocol",
"private": true,
"main": "./objectstack.config.ts",
"types": "./objectstack.config.ts",
"exports": {
".": "./objectstack.config.ts",
"./objectstack.config": "./objectstack.config.ts"
},
"scripts": {
"dev": "objectstack dev",
"build": "objectstack compile objectstack.config.ts dist/crm.json",
Expand Down
2 changes: 1 addition & 1 deletion examples/app-react-crud/src/mocks/browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ObjectQLPlugin } from '@objectstack/objectql';
import { InMemoryDriver } from '@objectstack/driver-memory';
import { MSWPlugin } from '@objectstack/plugin-msw';
// import appConfig from '../../objectstack.config';
import todoConfig from '@objectstack/example-todo/objectstack.config';
import todoConfig from '@example/app-todo/objectstack.config';

let kernel: ObjectKernel | null = null;

Expand Down
6 changes: 6 additions & 0 deletions examples/app-todo/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
"version": "0.9.0",
"description": "Example Todo App using ObjectStack Protocol",
"private": true,
"main": "./objectstack.config.ts",
"types": "./objectstack.config.ts",
"exports": {
".": "./objectstack.config.ts",
"./objectstack.config": "./objectstack.config.ts"
},
"scripts": {
"dev": "objectstack dev",
"build": "objectstack compile objectstack.config.ts dist/objectstack.json",
Expand Down
5 changes: 5 additions & 0 deletions examples/plugin-bi/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
"name": "@example/plugin-bi",
"version": "0.9.0",
"main": "src/index.ts",
"types": "src/index.ts",
"exports": {
".": "./src/index.ts",
"./objectstack.config": "./objectstack.config.ts"
},
Comment on lines 4 to +9
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Inconsistent package exports configuration. The "main" field points to "src/index.ts" (a plugin implementation), while the "exports" includes "./objectstack.config" pointing to "objectstack.config.ts" (a manifest). This differs from the pattern used in app-todo and app-crm where "main" points to "objectstack.config.ts".

Since app-host imports from "@example/plugin-bi/objectstack.config", the exports configuration is correct, but the "main" field should also point to "objectstack.config.ts" for consistency, or the package should clarify its dual purpose (both plugin implementation and manifest export).

Copilot uses AI. Check for mistakes.
"license": "MIT",
"private": true,
"scripts": {
Expand Down
13 changes: 8 additions & 5 deletions packages/core/src/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import { createRequire } from 'module';
import type { LoggerConfig, LogLevel } from '@objectstack/spec/system';
import type { Logger } from '@objectstack/spec/contracts';

const require = createRequire(import.meta.url);

/**
* Universal Logger Implementation
*
Expand All @@ -24,6 +21,7 @@ export class ObjectLogger implements Logger {
private isNode: boolean;
private pinoLogger?: any; // Pino logger instance for Node.js
private pinoInstance?: any; // Base Pino instance for creating child loggers
private require?: any; // CommonJS require function for Node.js

constructor(config: Partial<LoggerConfig> = {}) {
// Detect runtime environment
Expand Down Expand Up @@ -56,8 +54,13 @@ export class ObjectLogger implements Logger {
if (!this.isNode) return;

try {
// Create require function dynamically for Node.js (avoids bundling issues in browser)
// @ts-ignore - dynamic import of Node.js module
const { createRequire } = eval('require("module")');
this.require = createRequire(import.meta.url);

// Synchronous import for Pino using createRequire (works in ESM)
Comment on lines +57 to 62
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Using eval() to dynamically load Node.js modules creates security and operational risks:

  1. Security: eval() can be exploited and violates Content Security Policy (CSP) with 'unsafe-eval' restrictions
  2. Runtime Failures: This code will fail in browser environments with strict CSP
  3. Maintainability: Static analysis tools cannot track these dynamic dependencies

Consider these alternatives:

  • Use conditional dynamic imports with try/catch: await import('module')
  • Configure Vite/Rollup to externalize Node.js built-ins for browser builds
  • Create separate build targets for Node.js and browser environments

The pattern is repeated from plugin-signature-verifier.ts, suggesting a systemic approach to this problem. A more robust, security-conscious solution should be implemented across the codebase.

Suggested change
// Create require function dynamically for Node.js (avoids bundling issues in browser)
// @ts-ignore - dynamic import of Node.js module
const { createRequire } = eval('require("module")');
this.require = createRequire(import.meta.url);
// Synchronous import for Pino using createRequire (works in ESM)
// Use CommonJS require directly when available (Node.js CJS environments)
if (typeof require !== 'undefined') {
// @ts-ignore - require is only available in Node.js CJS
this.require = require;
} else {
// In environments without require (e.g., Node ESM), fail gracefully and use console fallback
throw new Error('CommonJS require is not available');
}
// Synchronous import for Pino using CommonJS require

Copilot uses AI. Check for mistakes.
const pino = require('pino');
const pino = this.require('pino');

// Build Pino options
const pinoOptions: any = {
Expand All @@ -81,7 +84,7 @@ export class ObjectLogger implements Logger {
// Check if pino-pretty is available
let hasPretty = false;
try {
require.resolve('pino-pretty');
this.require.resolve('pino-pretty');
hasPretty = true;
} catch (e) {
// ignore
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/security/plugin-signature-verifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import type { PluginMetadata } from '../plugin-loader.js';
let cryptoModule: typeof import('crypto') | null = null;
if (typeof (globalThis as any).window === 'undefined') {
try {
// Dynamic import for Node.js crypto module
// eslint-disable-next-line @typescript-eslint/no-var-requires
cryptoModule = require('crypto');
// Dynamic import for Node.js crypto module (using eval to avoid bundling issues)
// @ts-ignore - dynamic require for Node.js
cryptoModule = eval('require("crypto")');
Comment on lines +8 to +10
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

Using eval() to dynamically load Node.js modules creates a security risk and violates Content Security Policy (CSP) in environments that enforce 'unsafe-eval' restrictions. While this approach works around bundler issues, it has operational implications:

  1. Security: eval() execution can be exploited if an attacker can control the string being evaluated, even though the current usage appears safe
  2. CSP Compliance: Browsers with strict CSP will block eval(), making this code fail in secure contexts
  3. Code Analysis: Static analysis tools and bundlers cannot track the dependency

Consider these alternatives:

  • Use conditional dynamic imports: await import('crypto') with proper error handling
  • Use Vite/Rollup's external configuration to exclude Node.js modules from browser builds
  • Create separate entry points for Node.js and browser environments

The current approach may work but has significant operational and security tradeoffs that should be documented or addressed with a more robust solution.

Copilot uses AI. Check for mistakes.
} catch {
// Crypto module not available (e.g., browser environment)
}
Expand Down
Loading