Skip to content

Commit 3bda22a

Browse files
nicknisinicknisi
committed
build analysis and plan
1 parent fbe76af commit 3bda22a

File tree

2 files changed

+648
-0
lines changed

2 files changed

+648
-0
lines changed

‎build_analysis_v8.md‎

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
# WorkOS Node.js SDK v8.0 Build System Analysis
2+
3+
## Executive Summary
4+
5+
The WorkOS Node.js SDK v8.0 introduces a significant change from TypeScript Compiler (TSC) to tsup for building dual CommonJS (CJS) and ES Module (ESM) bundles. While this modernizes the build system and provides better module format support, **there are critical issues that prevent the builds from working correctly** that need immediate attention.
6+
7+
## Critical Issues Found (Standalone Usage Only) ⚠️
8+
9+
### 1. Module Resolution Issues in Standalone Node.js Usage
10+
11+
**Problem**: The current tsup configuration generates builds that work with modern bundlers but fail in standalone Node.js usage.
12+
13+
**CJS Error (Standalone)**:
14+
```
15+
Error [ERR_REQUIRE_ESM]: require() of ES Module /path/to/lib/common/crypto/subtle-crypto-provider.js from /path/to/lib/index.cjs not supported.
16+
```
17+
18+
**ESM Error (Standalone)**:
19+
```
20+
Cannot find module '/path/to/lib/common/crypto/subtle-crypto-provider' imported from /path/to/lib/index.js
21+
```
22+
23+
**Root Cause**: With `bundle: false`, tsup preserves individual files but doesn't adjust import paths for different formats. CJS files are trying to `require()` ESM files (`.js` extension), and ESM files are importing without proper file extensions.
24+
25+
### 2. Why It Works with Bundlers (relative-deps, Next.js, etc.)
26+
27+
Modern bundlers like webpack, Next.js, and tools like `relative-deps`:
28+
- **Module Resolution**: Handle mixed module formats automatically
29+
- **Import Rewriting**: Resolve imports at build time rather than runtime
30+
- **Bundling**: Often bundle dependencies, bypassing the module resolution issues
31+
- **Smart Resolution**: Use package.json exports properly and resolve to the correct format
32+
33+
### 3. Impact Assessment
34+
35+
| Usage Context | Status | Reason |
36+
|---------------|--------|---------|
37+
| **Next.js/React Apps** | ✅ Works | Bundler handles module resolution |
38+
| **Webpack Projects** | ✅ Works | Webpack resolves imports at build time |
39+
| **Bundled Applications** | ✅ Works | Dependencies get bundled |
40+
| **Standalone Node.js** | ❌ Broken | Raw module resolution fails |
41+
| **Direct require/import** | ❌ Broken | No bundler to resolve conflicts |
42+
43+
## Build System Changes Analysis
44+
45+
### Old Build System (main branch)
46+
```json
47+
{
48+
"scripts": {
49+
"build": "tsc -p ."
50+
},
51+
"tsconfig.json": {
52+
"module": "commonjs",
53+
"target": "es6"
54+
}
55+
}
56+
```
57+
58+
- **Single format**: CommonJS only
59+
- **Simple**: Direct TypeScript compilation
60+
- **Working**: No module resolution issues
61+
62+
### New Build System (v8 branch)
63+
```json
64+
{
65+
"scripts": {
66+
"build": "tsup"
67+
},
68+
"tsup.config.ts": {
69+
"format": ["cjs", "esm"],
70+
"bundle": false,
71+
"target": "es2022"
72+
}
73+
}
74+
```
75+
76+
- **Dual format**: Both CJS and ESM
77+
- **Complex**: Advanced bundler configuration
78+
- **Broken**: Module resolution issues
79+
80+
## Package.json Changes Analysis
81+
82+
### Export Map Evolution
83+
84+
**Old (v7)**:
85+
```json
86+
{
87+
"typings": "lib/index.d.ts",
88+
"exports": {
89+
"types": "./lib/index.d.ts",
90+
"workerd": {
91+
"import": "./lib/index.worker.js",
92+
"default": "./lib/index.worker.js"
93+
},
94+
"edge-light": {
95+
"import": "./lib/index.worker.js",
96+
"default": "./lib/index.worker.js"
97+
},
98+
"default": {
99+
"import": "./lib/index.js",
100+
"default": "./lib/index.js"
101+
}
102+
}
103+
}
104+
```
105+
106+
**New (v8)**:
107+
```json
108+
{
109+
"type": "module",
110+
"main": "./lib/index.cjs",
111+
"module": "./lib/index.js",
112+
"types": "./lib/index.d.ts",
113+
"exports": {
114+
".": {
115+
"types": "./lib/index.d.ts",
116+
"workerd": {
117+
"import": "./lib/index.worker.js",
118+
"require": "./lib/index.worker.cjs"
119+
},
120+
"edge-light": {
121+
"import": "./lib/index.worker.js",
122+
"require": "./lib/index.worker.cjs"
123+
},
124+
"import": "./lib/index.js",
125+
"require": "./lib/index.cjs",
126+
"default": "./lib/index.js"
127+
},
128+
"./worker": {
129+
"types": "./lib/index.worker.d.ts",
130+
"import": "./lib/index.worker.js",
131+
"require": "./lib/index.worker.cjs",
132+
"default": "./lib/index.worker.js"
133+
},
134+
"./package.json": "./package.json"
135+
}
136+
}
137+
```
138+
139+
### Key Changes Impact
140+
141+
1. **`"type": "module"`** - Package is now ESM-first
142+
2. **Dual build support** - Both CJS and ESM files available
143+
3. **Better runtime targeting** - Proper environment-specific exports
144+
4. **Backward compatibility fields** - `main`, `module`, `types` for older tooling
145+
146+
**User Impact**: The export map structure is well-designed and follows Node.js best practices, but the actual files don't work due to build issues.
147+
148+
## Environment Support Analysis
149+
150+
### Target Environments
151+
152+
| Environment | Old Support | New Support | Status |
153+
|-------------|-------------|-------------|---------|
154+
| **Bundled Apps (Next.js, webpack)** | ✅ | ✅ | Working |
155+
| **Node.js CJS (standalone)** | ✅ | ❌ (broken) | Regression |
156+
| **Node.js ESM (standalone)** | ❌ | ❌ (broken) | No improvement |
157+
| **Cloudflare Workers (bundled)** | ✅ | ✅ | Working |
158+
| **Edge Runtime (bundled)** | ✅ | ✅ | Working |
159+
160+
### TypeScript Definitions ✅
161+
162+
The build correctly generates TypeScript definitions for both formats:
163+
- `index.d.ts` (ESM definitions)
164+
- `index.d.cts` (CJS definitions)
165+
- `index.worker.d.ts` (Worker ESM definitions)
166+
- `index.worker.d.cts` (Worker CJS definitions)
167+
168+
## Potential User Impact
169+
170+
### Breaking Changes for Existing Users
171+
172+
1. **Package type change**: `"type": "module"` may affect tooling that doesn't handle package.json type field properly
173+
2. **File extension changes**: Users importing specific files may need to update import paths
174+
3. **Build tool compatibility**: Some bundlers might handle the new export map differently
175+
176+
### Migration Considerations
177+
178+
Once the build issues are fixed, users should:
179+
180+
1. **Test imports**: Verify both `require()` and `import` work
181+
2. **Check bundlers**: Ensure webpack, Rollup, esbuild handle new exports correctly
182+
3. **Validate TypeScript**: Confirm type resolution works in their projects
183+
4. **Test environments**: Verify functionality in Node.js, workers, and edge runtimes
184+
185+
## Recommendations
186+
187+
### Immediate Actions Required ⚠️
188+
189+
1. **Fix tsup configuration** to properly handle cross-format imports:
190+
```typescript
191+
// Option 1: Enable bundling
192+
bundle: true,
193+
194+
// Option 2: Configure proper extension handling
195+
esbuildOptions(options, { format }) {
196+
if (format === 'cjs') {
197+
options.mainFields = ['main', 'module']
198+
}
199+
}
200+
```
201+
202+
2. **Add import extension rewriting** for ESM builds
203+
3. **Test both formats** before release
204+
4. **Add integration tests** for both CJS and ESM consumption
205+
206+
### Testing Strategy
207+
208+
```bash
209+
# Test CJS consumption
210+
node -e "const { WorkOS } = require('@workos-inc/node'); console.log(typeof WorkOS);"
211+
212+
# Test ESM consumption
213+
node -e "import('@workos-inc/node').then(({ WorkOS }) => console.log(typeof WorkOS));"
214+
215+
# Test worker imports
216+
node -e "const { WorkOS } = require('@workos-inc/node/worker'); console.log(typeof WorkOS);"
217+
```
218+
219+
### Long-term Improvements
220+
221+
1. **Add build validation** to CI/CD pipeline
222+
2. **Include module resolution tests** in test suite
223+
3. **Document migration guide** for users upgrading from v7
224+
4. **Consider bundling** for simpler distribution
225+
226+
## Risk Assessment
227+
228+
| Risk Level | Issue | Impact |
229+
|------------|-------|---------|
230+
| 🟡 **Medium** | Standalone Node.js usage broken | CLI tools, direct usage affected |
231+
| 🟡 **Medium** | Package type change | Potential tooling compatibility issues |
232+
| 🟢 **Low** | Export map changes | Minimal impact, follows standards |
233+
| 🟢 **Low** | Bundled app usage | Works fine with modern bundlers |
234+
235+
## Conclusion
236+
237+
The v8 build system modernization is a positive direction with proper dual-format support and better environment targeting. **The package works correctly in most real-world usage scenarios** (bundled applications, Next.js, webpack projects) but has module resolution issues in standalone Node.js usage.
238+
239+
### Key Findings:
240+
241+
1. **✅ Works with bundlers**: Modern build tools handle the module resolution correctly
242+
2. **❌ Broken for standalone**: Direct Node.js require/import fails due to cross-format imports
243+
3. **✅ Good architecture**: Export map and TypeScript definitions are well-designed
244+
4. **🟡 Limited impact**: Most users won't encounter the standalone issues
245+
246+
### Updated Recommendation:
247+
248+
The v8.0 release can proceed for most use cases, but should include:
249+
- **Documentation** warning about standalone usage limitations
250+
- **Future fix** for tsup configuration to support standalone usage
251+
- **Testing** of both bundled and standalone scenarios
252+
253+
**For most users**: The package will work fine in their applications.
254+
**For CLI/standalone users**: May need to wait for a future patch or use dynamic imports as workarounds.

0 commit comments

Comments
 (0)