Skip to content

Commit 395e0e8

Browse files
committed
fix: simplify async helper loading and fix container tests
- Simplified async helper loading by properly awaiting requireHelperFromModule - Removed redundant Promise checking and async chain handling - Fixed container tests to properly await Container.create() - Reduced test failures from 4 to 2 (remaining 2 are pre-existing) - All _init calls now happen in a single loop after helpers are loaded Test results: 482 passed, 2 failed (pre-existing), 16 skipped
1 parent 6c5537b commit 395e0e8

File tree

2 files changed

+94
-113
lines changed

2 files changed

+94
-113
lines changed

lib/container.js

Lines changed: 80 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ async function ensureTsxLoader() {
3131
const { createRequire } = await import('module')
3232
const { pathToFileURL } = await import('url')
3333
const userRequire = createRequire(pathToFileURL(path.join(global.codecept_dir || process.cwd(), 'package.json')).href)
34-
34+
3535
// Try to resolve tsx from user's project
3636
try {
3737
userRequire.resolve('tsx')
@@ -77,7 +77,7 @@ let container = {
7777
translation: {},
7878
/** @type {Result | null} */
7979
result: null,
80-
sharedKeys: new Set() // Track keys shared via share() function
80+
sharedKeys: new Set(), // Track keys shared via share() function
8181
}
8282

8383
/**
@@ -295,10 +295,10 @@ class Container {
295295
// Instead of using append which replaces the entire container,
296296
// directly update the support object to maintain proxy references
297297
Object.assign(container.support, data)
298-
298+
299299
// Track which keys were explicitly shared
300300
Object.keys(data).forEach(key => container.sharedKeys.add(key))
301-
301+
302302
if (!options.local) {
303303
WorkerStorage.share(data)
304304
}
@@ -335,33 +335,13 @@ async function createHelpers(config) {
335335

336336
// classical require - may be async for ESM modules
337337
if (!HelperClass) {
338-
const helperResult = requireHelperFromModule(helperName, config)
339-
if (helperResult instanceof Promise) {
340-
// Handle async ESM loading
341-
helpers[helperName] = {}
342-
asyncHelperPromise = asyncHelperPromise
343-
.then(() => helperResult)
344-
.then(async ResolvedHelperClass => {
345-
debug(`helper ${helperName} resolved type: ${typeof ResolvedHelperClass}`, ResolvedHelperClass)
346-
347-
// Extract default export from ESM module wrapper if needed
348-
if (ResolvedHelperClass && ResolvedHelperClass.__esModule && ResolvedHelperClass.default) {
349-
ResolvedHelperClass = ResolvedHelperClass.default
350-
debug(`extracted default export for ${helperName}, new type: ${typeof ResolvedHelperClass}`)
351-
}
338+
// requireHelperFromModule is async, so we need to await it
339+
HelperClass = await requireHelperFromModule(helperName, config)
352340

353-
if (typeof ResolvedHelperClass !== 'function') {
354-
throw new Error(`Helper '${helperName}' is not a class. Got: ${typeof ResolvedHelperClass}`)
355-
}
356-
357-
checkHelperRequirements(ResolvedHelperClass)
358-
helpers[helperName] = new ResolvedHelperClass(config[helperName])
359-
if (helpers[helperName]._init) await helpers[helperName]._init()
360-
debug(`helper ${helperName} async initialized`)
361-
})
362-
continue
363-
} else {
364-
HelperClass = helperResult
341+
// Extract default export from ESM module wrapper if needed
342+
if (HelperClass && HelperClass.__esModule && HelperClass.default) {
343+
HelperClass = HelperClass.default
344+
debug(`extracted default export for ${helperName}`)
365345
}
366346
}
367347

@@ -396,7 +376,7 @@ async function createHelpers(config) {
396376

397377
// Wait for all async helpers to be resolved before calling _init
398378
await asyncHelperPromise
399-
379+
400380
for (const name in helpers) {
401381
if (helpers[name]._init) await helpers[name]._init()
402382
}
@@ -441,81 +421,83 @@ async function requireHelperFromModule(helperName, config, HelperClass) {
441421
HelperClass = mod.default || mod
442422
} catch (err) {
443423
if (err.code === 'ERR_UNKNOWN_FILE_EXTENSION' || (err.message && err.message.includes('Unknown file extension'))) {
444-
// This is likely a TypeScript helper file. Transpile it to a temporary JS file
445-
// and import that as a reliable fallback (no NODE_OPTIONS required).
446-
try {
447-
const pathModule = await import('path')
448-
const absolutePath = pathModule.default.resolve(moduleName)
424+
// This is likely a TypeScript helper file. Transpile it to a temporary JS file
425+
// and import that as a reliable fallback (no NODE_OPTIONS required).
426+
try {
427+
const pathModule = await import('path')
428+
const absolutePath = pathModule.default.resolve(moduleName)
449429

450-
// Attempt to load local 'typescript' to transpile the helper
451-
let typescript
452-
try {
453-
typescript = await import('typescript')
454-
} catch (tsImportErr) {
455-
throw new Error(`TypeScript helper detected (${moduleName}). Please install 'typescript' in your project: npm install --save-dev typescript`)
456-
}
430+
// Attempt to load local 'typescript' to transpile the helper
431+
let typescript
432+
try {
433+
typescript = await import('typescript')
434+
} catch (tsImportErr) {
435+
throw new Error(`TypeScript helper detected (${moduleName}). Please install 'typescript' in your project: npm install --save-dev typescript`)
436+
}
457437

458-
const { tempFile, allTempFiles } = await transpileTypeScript(absolutePath, typescript)
459-
debug(`Transpiled TypeScript helper: ${moduleName} -> ${tempFile}`)
438+
const { tempFile, allTempFiles } = await transpileTypeScript(absolutePath, typescript)
439+
debug(`Transpiled TypeScript helper: ${moduleName} -> ${tempFile}`)
460440

441+
try {
461442
try {
462-
try {
463-
const mod = await import(tempFile)
464-
HelperClass = mod.default || mod
465-
debug(`helper ${helperName} loaded from transpiled JS: ${tempFile}`)
466-
} catch (importTempErr) {
467-
// If import fails due to CommonJS named export incompatibility,
468-
// try a quick transform: convert named imports from CommonJS packages
469-
// into default import + destructuring. This resolves cases like
470-
// "Named export 'Helper' not found. The requested module '@codeceptjs/helper' is a CommonJS module"
471-
const msg = importTempErr && importTempErr.message || ''
472-
const commonJsMatch = msg.match(/The requested module '(.+?)' is a CommonJS module/)
473-
if (commonJsMatch) {
474-
// Read the transpiled file, perform heuristic replacement, and import again
475-
const fs = await import('fs')
476-
let content = fs.readFileSync(tempFile, 'utf8')
477-
// Heuristic: replace "import { X, Y } from 'mod'" with default import + destructure
478-
content = content.replace(/import\s+\{([^}]+)\}\s+from\s+(['"])([^'"\)]+)\2/gm, (m, names, q, modName) => {
479-
// Only adjust imports for the module reported in the error or for local modules
480-
if (modName === commonJsMatch[1] || modName.startsWith('.') || !modName.includes('/')) {
481-
const cleanedNames = names.trim()
482-
return `import pkg__interop from ${q}${modName}${q};\nconst { ${cleanedNames} } = pkg__interop`
483-
}
484-
return m
485-
})
486-
487-
// Write to a secondary temp file
488-
const os = await import('os')
489-
const path = await import('path')
490-
const fallbackTemp = path.default.join(os.default.tmpdir(), `helper-fallback-${Date.now()}.mjs`)
491-
fs.writeFileSync(fallbackTemp, content, 'utf8')
492-
try {
493-
const mod = await import(fallbackTemp)
494-
HelperClass = mod.default || mod
495-
debug(`helper ${helperName} loaded from transpiled JS after CommonJS interop fix: ${fallbackTemp}`)
496-
} finally {
497-
try { fs.unlinkSync(fallbackTemp) } catch (e) {}
443+
const mod = await import(tempFile)
444+
HelperClass = mod.default || mod
445+
debug(`helper ${helperName} loaded from transpiled JS: ${tempFile}`)
446+
} catch (importTempErr) {
447+
// If import fails due to CommonJS named export incompatibility,
448+
// try a quick transform: convert named imports from CommonJS packages
449+
// into default import + destructuring. This resolves cases like
450+
// "Named export 'Helper' not found. The requested module '@codeceptjs/helper' is a CommonJS module"
451+
const msg = (importTempErr && importTempErr.message) || ''
452+
const commonJsMatch = msg.match(/The requested module '(.+?)' is a CommonJS module/)
453+
if (commonJsMatch) {
454+
// Read the transpiled file, perform heuristic replacement, and import again
455+
const fs = await import('fs')
456+
let content = fs.readFileSync(tempFile, 'utf8')
457+
// Heuristic: replace "import { X, Y } from 'mod'" with default import + destructure
458+
content = content.replace(/import\s+\{([^}]+)\}\s+from\s+(['"])([^'"\)]+)\2/gm, (m, names, q, modName) => {
459+
// Only adjust imports for the module reported in the error or for local modules
460+
if (modName === commonJsMatch[1] || modName.startsWith('.') || !modName.includes('/')) {
461+
const cleanedNames = names.trim()
462+
return `import pkg__interop from ${q}${modName}${q};\nconst { ${cleanedNames} } = pkg__interop`
498463
}
499-
} else {
500-
throw importTempErr
464+
return m
465+
})
466+
467+
// Write to a secondary temp file
468+
const os = await import('os')
469+
const path = await import('path')
470+
const fallbackTemp = path.default.join(os.default.tmpdir(), `helper-fallback-${Date.now()}.mjs`)
471+
fs.writeFileSync(fallbackTemp, content, 'utf8')
472+
try {
473+
const mod = await import(fallbackTemp)
474+
HelperClass = mod.default || mod
475+
debug(`helper ${helperName} loaded from transpiled JS after CommonJS interop fix: ${fallbackTemp}`)
476+
} finally {
477+
try {
478+
fs.unlinkSync(fallbackTemp)
479+
} catch (e) {}
501480
}
481+
} else {
482+
throw importTempErr
502483
}
503-
} finally {
504-
// Cleanup transpiled temporary files
505-
const filesToClean = Array.isArray(allTempFiles) ? allTempFiles : [allTempFiles]
506-
cleanupTempFiles(filesToClean)
507484
}
508-
} catch (importErr) {
509-
throw new Error(
510-
`Helper '${helperName}' is a TypeScript file but could not be loaded.\n` +
485+
} finally {
486+
// Cleanup transpiled temporary files
487+
const filesToClean = Array.isArray(allTempFiles) ? allTempFiles : [allTempFiles]
488+
cleanupTempFiles(filesToClean)
489+
}
490+
} catch (importErr) {
491+
throw new Error(
492+
`Helper '${helperName}' is a TypeScript file but could not be loaded.\n` +
511493
`Path: ${moduleName}\n` +
512494
`Error: ${importErr.message}\n\n` +
513495
`To load TypeScript helpers, install 'typescript' in your project or use an ESM loader (e.g. tsx):\n` +
514496
` npm install --save-dev typescript\n` +
515497
` OR run CodeceptJS with an ESM loader: NODE_OPTIONS='--import tsx' npx codeceptjs run\n\n` +
516-
`CodeceptJS will transpile TypeScript helpers automatically at runtime if 'typescript' is available.`
517-
)
518-
}
498+
`CodeceptJS will transpile TypeScript helpers automatically at runtime if 'typescript' is available.`,
499+
)
500+
}
519501
} else if (err.code === 'ERR_REQUIRE_ESM' || (err.message && err.message.includes('ES module'))) {
520502
// This is an ESM module, use dynamic import
521503
try {
@@ -801,24 +783,23 @@ async function loadSupportObject(modulePath, supportObjectName) {
801783
// Use dynamic import for both ESM and CJS modules
802784
let importPath = modulePath
803785
let tempJsFile = null
804-
786+
805787
if (typeof importPath === 'string') {
806788
const ext = path.extname(importPath)
807-
789+
808790
// Handle TypeScript files
809791
if (ext === '.ts') {
810792
try {
811793
// Use the TypeScript transpilation utility
812794
const typescript = await import('typescript')
813795
const { tempFile, allTempFiles } = await transpileTypeScript(importPath, typescript)
814-
796+
815797
debug(`Transpiled TypeScript file: ${importPath} -> ${tempFile}`)
816-
798+
817799
// Attach cleanup handler
818800
importPath = tempFile
819801
// Store temp files list in a way that cleanup can access them
820802
tempJsFile = allTempFiles
821-
822803
} catch (tsError) {
823804
throw new Error(`Failed to load TypeScript file ${importPath}: ${tsError.message}. Make sure 'typescript' package is installed.`)
824805
}
@@ -827,7 +808,7 @@ async function loadSupportObject(modulePath, supportObjectName) {
827808
importPath = `${importPath}.js`
828809
}
829810
}
830-
811+
831812
let obj
832813
try {
833814
obj = await import(importPath)

test/unit/container_test.js

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ describe('Container', () => {
161161
FileSystem: {},
162162
},
163163
}
164-
container.create(config)
164+
await container.create(config)
165165
await container.started()
166166
// custom helpers
167167
expect(container.helpers('MyHelper')).is.ok
@@ -266,7 +266,7 @@ describe('Container', () => {
266266
FileSystem: {},
267267
},
268268
}
269-
container.create(config)
269+
await container.create(config)
270270
await container.started()
271271
container.append({
272272
helpers: {
@@ -294,10 +294,10 @@ describe('Container', () => {
294294
const tsStepsPath = path.join(__dirname, '../data/typescript-support/steps_file.ts')
295295
await container.create({
296296
include: {
297-
I: tsStepsPath
298-
}
297+
I: tsStepsPath,
298+
},
299299
})
300-
300+
301301
const I = container.support('I')
302302
expect(I).to.be.ok
303303
expect(I.testMethod).to.be.a('function')
@@ -309,10 +309,10 @@ describe('Container', () => {
309309
const tsStepsPath = path.join(__dirname, '../data/typescript-support/steps_file.ts')
310310
await container.create({
311311
include: {
312-
I: tsStepsPath
313-
}
312+
I: tsStepsPath,
313+
},
314314
})
315-
315+
316316
const I = container.support('I')
317317
// Note: These are proxied through MetaStep, so we can't call them directly in tests
318318
// The test verifies that the file loads and the structure is correct
@@ -325,10 +325,10 @@ describe('Container', () => {
325325
const tsStepsPath = path.join(__dirname, '../data/typescript-support/steps_with_dirname.ts')
326326
await container.create({
327327
include: {
328-
I: tsStepsPath
329-
}
328+
I: tsStepsPath,
329+
},
330330
})
331-
331+
332332
const I = container.support('I')
333333
expect(I).to.be.ok
334334
expect(I.getConfigPath).to.be.a('function')
@@ -340,10 +340,10 @@ describe('Container', () => {
340340
const tsStepsPath = path.join(__dirname, '../data/typescript-support/steps_with_require.ts')
341341
await container.create({
342342
include: {
343-
I: tsStepsPath
344-
}
343+
I: tsStepsPath,
344+
},
345345
})
346-
346+
347347
const I = container.support('I')
348348
expect(I).to.be.ok
349349
expect(I.getPluginPath).to.be.a('function')

0 commit comments

Comments
 (0)