Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Commit

Permalink
fix: Support class features when transforming source code
Browse files Browse the repository at this point in the history
appmap-agent-js uses @babel/parser to parse the ECMAScript code for
transformation; the output generated by that library is not 100%
estree-compliant [1]. It has an `estree` plugin that reverts the
deviations; however, by default not all of them are supported
(apparently for @babel/parser's own backward compatibility reasons).
Fortunately it has a config setting that enables them.

One of these differences is that the standard node type of
PropertyDefinition is called ClassProperty instead, which made the
transformer unable to correctly handle ES classes.

[1] https://babeljs.io/docs/babel-parser#output

Fixes #199
  • Loading branch information
dividedmind committed Mar 3, 2023
1 parent 5f8ab1c commit b7760c7
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 4 deletions.
2 changes: 1 addition & 1 deletion component/signature.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const parseBabelLog = (content, url) => {
return parseBabel(content, {
sourceType: "module",
sourceFilename: url,
plugins: ["estree"],
plugins: [["estree", { classFeatures: true }]],
}).program;
} catch (error) {
error.message += ` at ${url}`;
Expand Down
2 changes: 1 addition & 1 deletion components/instrumentation/default/__fixture__.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ export const normalize = (code, source) =>
ecmaVersion: 2021,
sourceType: source,
allowAwaitOutsideFunction: source === "module",
plugins: ["estree"],
plugins: [["estree", { classFeatures: true }]],
}).program,
);
15 changes: 15 additions & 0 deletions components/instrumentation/default/index.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,21 @@ const instrumentation = createInstrumentation(
);
}

{
const js = "class Klass { prop; }";
const source = createSource("protocol://host/base/foo.js", js);
assertDeepEqual(
normalizeContent(
instrument(instrumentation, source, createMirrorMapping(source)),
),
{
url: "protocol://host/base/foo.js",
content: normalize(js, "script"),
sources: [source],
},
);
}

{
const source = createSource("protocol://host/base/bar.js", "456;");
assertDeepEqual(
Expand Down
2 changes: 1 addition & 1 deletion components/source/default/parse.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const parseEstree = (url, content) => {
} else if (/^[ \t\n]*\/(\/[ \t]*|\*[ \t\n]*)@flow/u.test(content)) {
plugins = ["flow"];
}
plugins.push("estree", "jsx");
plugins.push(["estree", { classFeatures: true }], "jsx");
let result;
try {
result = parseBabel(content, {
Expand Down
12 changes: 11 additions & 1 deletion components/source/default/parse.test.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { assertDeepEqual } from "../../__fixture__.mjs";
import { assertDeepEqual, assertEqual } from "../../__fixture__.mjs";
import {
parseEstree,
printComment,
Expand Down Expand Up @@ -40,6 +40,16 @@ assertDeepEqual(
},
);

{
const parsedClass = parseEstree(
"protocol://host/dirname/filename.js?search#hash",
"class Foo { prop; }",
);

// make sure property definitions have estree-compliant type
assertEqual(parsedClass.body[0].body.body[0].type, "PropertyDefinition");
}

assertDeepEqual(
getLeadingCommentArray(
parseEstree(
Expand Down

0 comments on commit b7760c7

Please sign in to comment.