Skip to content

better error handling when loading tasks using esm #133

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.

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[*]
indent_size = 2
indent_style = space
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test/fixtures
7 changes: 6 additions & 1 deletion .mocharc.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
{
"require": ["./env.js"],
"extension": ["ts"],
"recursive": true
"recursive": true,
"exclude": "test/fixtures/**",
"node-option": [
"experimental-specifier-resolution=node",
"loader=ts-node/esm"
]
}
24 changes: 14 additions & 10 deletions src/asyncAssignTasks.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { readdirSync } from 'fs'
import path from 'path'
import { pathToFileURL } from 'url'

export default async function asyncAssignTasks<T>(thisObj: T, tasksDir: string): Promise<void> {
if (!tasksDir) throw new Error(`tasksDir was ${tasksDir}`)
Expand All @@ -16,16 +17,19 @@ export default async function asyncAssignTasks<T>(thisObj: T, tasksDir: string):
}

async function loadTask(name: string) {
const path = `${tasksDir}/${name}`
try {
// Typescript has issues with dynamic imports, even though node supports them
// for both commonjs and esmodules. see: https://github.com/microsoft/TypeScript/issues/43329
const task = await Function(`return import("${path}")`)()
return task[name]
} catch (err) {
return () => {
throw new Error(`No task in: ${path}.{ts,js,tsx,jsx}`)
}
// this is needed for Windows, or you get ERR_UNSUPPORTED_ESM_URL_SCHEME
const path = pathToFileURL(`${tasksDir}/${name}`).href

// Typescript has issues with dynamic imports, even though node supports them
// for both commonjs and esmodules. see: https://github.com/microsoft/TypeScript/issues/43329
const task = await Function(`return import("${path}")`)()

// if typescript is using commonjs modules
if (task.default) {
return task.default[name]
}

// if typescript is using es modules
return task[name]
}
}
38 changes: 38 additions & 0 deletions test/asyncTasks.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import assert = require('assert')
import { Action, ActorWorld } from '../src'
import type { IActorWorldOptions } from '../src/ActorWorld'
import asyncAssignTasks from '../src/asyncAssignTasks'

describe(asyncAssignTasks.name, () => {
const options: IActorWorldOptions = { parameters: {} } as IActorWorldOptions

// success
it('successfully loads a task', async () => {
class TestWorld extends ActorWorld {
public goodtask: () => Action<void>
}

const world = new TestWorld(options)

// Load the task
await asyncAssignTasks(world, `${__dirname}/fixtures/tasks/goodtasks`)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand something here.

On line 12 we've defined goodtask as () => Action<void> but that's also what's defined in the goodtask.ts fixture.

Should we be using the result of this call to asynAssignTasks to set up the goodtask property on our TestWorld on line 12, instead of hard-coding / duplicating it?


// Execute the task
world.goodtask()
})

// error compiling
it('fails with a helpful error message if the task could not be compiled', async () => {
class TestWorld extends ActorWorld {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need lines 26-30 for this test?

public brokentask: () => Action<void>
}

const world = new TestWorld(options)

// Try to load the task
await assert.rejects(asyncAssignTasks(world, `${__dirname}/fixtures/tasks/brokentask`), {
name: 'TSError',
message: /Cannot find module '\.\/src'/,
})
})
})
7 changes: 7 additions & 0 deletions test/fixtures/tasks/brokentask/brokentask.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// this import will fail
import { type Action } from './src'

export const badtask = (): Action => {
// eslint-disable-next-line @typescript-eslint/no-empty-function
return () => {}
}
6 changes: 6 additions & 0 deletions test/fixtures/tasks/goodtasks/goodtask.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { type Action } from '../../../../src'

export const goodtask = (): Action<void> => {
// eslint-disable-next-line @typescript-eslint/no-empty-function
return () => {}
}
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
"strictNullChecks": true,
"noImplicitReturns": true
},
"include": ["src", "test"]
"include": ["src", "test"],
"exclude": ["test/fixtures"]
}