Skip to content
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

Sourcemapping #848

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 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
14 changes: 10 additions & 4 deletions packages/compiler/src/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const defaultLogger = createLogger();
// }

export type Options = TransformOptions & {
name?: string;
logger?: Logger;
logCompiledSource?: boolean;
};
Expand All @@ -27,15 +28,20 @@ export default function compile(pathOrSource: string, options: Options = {}) {
} else {
//logger.debug('Starting compilation from string');
}
const ast = parse(source);

const name = options.name ?? 'src';
const ast = parse(source, { name });

const transformedAst = transform(ast, undefined, options);

const compiledSource = print(transformedAst).code;
const { code, map } = print(transformedAst, {
sourceMapName: `${name}.map.js`,
});

if (options.logCompiledSource) {
logger.debug('Compiled source:');
logger.debug(compiledSource); // TODO indent or something
logger.debug(code);
}

return compiledSource;
return { code, map };
}
8 changes: 7 additions & 1 deletion packages/compiler/src/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@
import recast from 'recast';
import * as acorn from 'acorn';

export default function parse(source: string) {
type Options = {
/** Name of the source job (no file extension). This triggers source map generation */
name?: string;
};

export default function parse(source: string, options: Options = {}) {
// This is copied from v1 but I am unsure the usecase
const escaped = source.replace(/\ $/, '');

const ast = recast.parse(escaped, {
sourceFileName: options.name && `${options.name}.js`,
tolerant: true,
range: true,
parser: {
Expand Down
46 changes: 28 additions & 18 deletions packages/compiler/test/compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,63 @@ import fs from 'node:fs/promises';
import path from 'node:path';
import compile from '../src/compile';

// Not doing deep testing on this because recast does the heavy lifting
// This is just to ensure the map is actually generated
test('generate a source map if a file name is passed', (t) => {
const source = 'fn();';
const { map } = compile(source, { name: 'job' });
t.truthy(map);
t.deepEqual(map.sources, ['job.js']);
t.is(map.file, 'job.map.js');
});

test('ensure default exports is created', (t) => {
const source = '';
const expected = 'export default [];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('do not add default exports if exports exist', (t) => {
const source = 'export const x = 10;';
const expected = 'export const x = 10;';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a single operation', (t) => {
const source = 'fn();';
const expected = 'export default [fn()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a single namespaced operation', (t) => {
const source = 'http.get();';
const expected = 'export default [http.get()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a const assignment with single method call', (t) => {
const source = 'const x = dateFns.parse()';
const expected = `const x = dateFns.parse()
export default [];`;
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a single operation without being fussy about semicolons', (t) => {
const source = 'fn()';
const expected = 'export default [fn()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile multiple operations', (t) => {
const source = 'fn();fn();fn();';
const expected = 'export default [fn(), fn(), fn()];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

Expand All @@ -66,7 +76,7 @@ test('add imports', (t) => {
};
const source = 'fn();';
const expected = `import { fn } from "@openfn/language-common";\nexport default [fn()];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -84,7 +94,7 @@ test('do not add imports', (t) => {
// This example already has the correct imports declared, so add-imports should do nothing
const source = "import { fn } from '@openfn/language-common'; fn();";
const expected = `import { fn } from '@openfn/language-common';\nexport default [fn()];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -101,7 +111,7 @@ test('dumbly add imports', (t) => {
// This example already has the correct imports declared, so add-imports should do nothing
const source = "import { jam } from '@openfn/language-common'; jam(state);";
const expected = `import { jam } from '@openfn/language-common';\nexport default [jam(state)];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -119,7 +129,7 @@ test('add imports with export all', (t) => {
};
const source = 'fn();';
const expected = `import { fn } from "@openfn/language-common";\nexport * from "@openfn/language-common";\nexport default [fn()];`;
const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -134,28 +144,28 @@ test('twitter example', async (t) => {
path.resolve('test/jobs/twitter.compiled.js'),
'utf8'
);
const result = compile(source);
const { code: result } = compile(source);
t.deepEqual(result, expected);
});

test('compile with optional chaining', (t) => {
const source = 'fn(a.b?.c);';
const expected = 'export default [fn(a.b?.c)];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile with nullish coalescence', (t) => {
const source = 'fn(a ?? b);';
const expected = 'export default [fn(a ?? b)];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

test('compile a lazy state ($) expression', (t) => {
const source = 'get($.data.endpoint);';
const expected = 'export default [get(state => state.data.endpoint)];';
const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

Expand All @@ -175,7 +185,7 @@ test('compile a lazy state ($) expression with dumb imports', (t) => {
export * from "@openfn/language-common";
export default [get(state => state.data.endpoint)];`;

const result = compile(source, options);
const { code: result } = compile(source, options);
t.is(result, expected);
});

Expand All @@ -190,7 +200,7 @@ export default [_defer(
p => p.then((s => { console.log(s.data); return state;} ))
)];`;

const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});

Expand All @@ -207,6 +217,6 @@ export default [each(
_defer(post("/upsert", (state) => state.data), p => p.then((s) => s))
)];`;

const result = compile(source);
const { code: result } = compile(source);
t.is(result, expected);
});
3 changes: 3 additions & 0 deletions packages/compiler/test/parse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import parse from '../src/parse';

import { loadAst } from './util';

// Note that these tests do not include source mappings
// just because the ast changes and develops circular structures

test('parse a simple statement', (t) => {
const source = 'const x = 10;';

Expand Down
6 changes: 5 additions & 1 deletion packages/runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@
"author": "Open Function Group <[email protected]>",
"license": "ISC",
"devDependencies": {
"@openfn/compiler": "workspace:^",
"@openfn/language-common": "2.0.0-rc3",
"@openfn/lexicon": "workspace:^",
"@types/mock-fs": "^4.13.1",
"@types/node": "^18.15.13",
"@types/semver": "^7.5.0",
"ava": "5.3.1",
"mock-fs": "^5.4.1",
"recast": "^0.21.5",
"ts-node": "^10.9.1",
"tslib": "^2.4.0",
"tsup": "^7.2.0",
"typescript": "^5.1.6"
Expand All @@ -45,6 +48,7 @@
"dependencies": {
"@openfn/logger": "workspace:*",
"fast-safe-stringify": "^2.1.1",
"semver": "^7.5.4"
"semver": "^7.5.4",
"source-map": "^0.7.4"
}
}
39 changes: 39 additions & 0 deletions packages/runtime/src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
// It would be nice for the detail to be in the error, not the code
// But that probably requires more detailed error types

import expression, { ExecuteBreak } from './execute/expression'

export function assertImportError(e: any) {
if (e.name === 'ImportError') {
throw e;
Expand Down Expand Up @@ -50,10 +52,28 @@ export function assertAdaptorError(e: any) {
}
}

// v8 only returns positional information as a string
// this function will pull the line/col information back out of it
export const extractCallSite = (e: RTError) => {
if (e.stack) {
debugger;
const [_message, frame1] = e.stack.split('\n');

// find the line:col at the end
// structures here https://nodejs.org/api/errors.html#errorstack
const parts = frame1.split(':');
e.pos = {
col: parseInt(parts.pop()!.replace(')', '')),
line: parseInt(parts.pop()!),
};
}
};

// Abstract error supertype
export class RTError extends Error {
source = 'runtime';
name: string = 'Error';
pos?: { col: number, line: number} = undefined;

constructor() {
super();
Expand Down Expand Up @@ -92,6 +112,13 @@ export class RuntimeError extends RTError {
super();
this.subtype = error.constructor.name;
this.message = `${this.subtype}: ${error.message}`;

// automatically limit the stacktrace (?)
// Error.captureStackTrace(this, expression);

// extract positional info for source mapping
extractCallSite(error);
this.pos = error.pos;
}
}

Expand All @@ -106,7 +133,19 @@ export class RuntimeCrash extends RTError {
constructor(error: Error) {
super();
this.subtype = error.constructor.name;
// this.type = error.type;
this.message = `${this.subtype}: ${error.message}`;
// this.stack = error.stack;

// automatically limit the stacktrace (?)
// console.log(ExecuteBreak)
// Error.captureStackTrace(error, ExecuteBreak);
// Error.captureStackTrace(error, undefined);
// Error.captureStackTrace(error, function(){});

// extract positional info for source mapping
extractCallSite(error);
this.pos = error.pos;
}
}

Expand Down
Loading