-
Notifications
You must be signed in to change notification settings - Fork 45
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
On test re-run, Wallaby is rewriting import
to require
#2818
Comments
Yes - By default for commonjs modules, Wallaby re-writes When Wallaby's esm loader is used the import statements are not touched but because you have a custom loader in place, this logic was missing. We have released an updated version of Wallaby core (
I think another way you may have been able to solve the problem is with: import { test } from 'ava';
Yes, we recently added support (in the last 2 weeks). You may also transpire them to CJS if you want. We had not expected anyone to use a custom loader, which I think is what has caused the problem in your case. We're keen to understand why you have this and to see a template for your ultimate setup. If we can automatically handle what you're doing we will (or at least update our docs for others who want to do the same thing). |
What changes were made? I still have to add this hook to my
I will setup a repro project now. |
We added support for using your own experimental loader, and not the Wallaby We will wait to see the repro project, thanks. |
Here is the repro: https://github.com/vjpr/issue-wallaby-ava Just install with Then run the It's using |
Sorry - missed this at the end of your first message. Yes - the ranges are necessary, it is how Wallaby maps your code. I've been taking a look at how to get the ranges with SWC. It may be possible with an SWC |
I have managed to get the ranges working by updating your swc compiler (see below). Right now, it's not how I would leave it in terms of efficiency, the I'm going to try see why that's happening now. import * as swc from "@swc/core";
import { Visitor } from "@swc/core/Visitor.js";
class RangeCapturer extends Visitor {
getAllFuncs(toCheck) {
const props = [];
let obj = toCheck;
do {
props.push(...Object.getOwnPropertyNames(obj));
} while ((obj = Object.getPrototypeOf(obj)));
return props.sort().filter((e, i, arr) => {
if (e != arr[i + 1] && typeof toCheck[e] == "function") return true;
});
}
constructor() {
super();
this.ranges = [];
this.getAllFuncs(this).forEach(funcName => {
if (funcName.startsWith('visit')) {
const oldVisit = this[funcName];
this[funcName] = function () {
if (arguments.length > 0) {
if (arguments[0] && arguments[0].span) {
this.ranges.push([arguments[0].span.start, arguments[0].span.end]);
}
}
return oldVisit.apply(this, arguments);
};
}
});
}
}
import { dirname } from "path";
////////////////////////////////////////////////////////////////////////////////
const swcConfig = {
test: ".tsx?$",
sourceMaps: true,
jsc: {
target: "es2020",
parser: {
syntax: "typescript",
tsx: true,
decorators: true,
dynamicImport: true,
},
baseUrl: "./src",
paths: {
//'@src/*': ['*'],
"@src/*": ["./src/*"],
"#src/*": ["./src/*"],
"#packages/*": ["./packages/*"],
},
},
module: {
//type: 'commonjs',
type: "es6",
},
};
////////////////////////////////////////////////////////////////////////////////
function rangeByIdxToLineColumn(source, idx) {
const content = source.substring(0, idx).replace(/\r\n/g, '\n').split('\n');
const line = content.length;
const column = content[content.length - 1].length;
return [line, column];
}
export default function (w, { shouldLog } = {}) {
return (file) => {
const { type, path, content, config } = file;
const filename = path;
if (shouldLog) console.log("Compiling:", filename);
const plugins = [];
const opts = {
sourceRoot: dirname(filename),
filename,
//jsc // Config options can be added here.
jsc: { ...swcConfig.jsc },
//plugin: swc.plugins(plugins),
};
const res = compile(filename, content, opts);
//console.log({filename, content, res})
return res;
};
}
////////////////////////////////////////////////////////////////////////////////
function compile(filename, code, opts) {
const defaultSourceMap = true;
//const defaultSourceMap = 'inline' // originally
const rangeCapturer = new RangeCapturer();
const finalOpts = {
...opts,
sourceMaps:
opts.sourceMaps === undefined ? defaultSourceMap : opts.sourceMaps,
plugin: (m) => rangeCapturer.visitProgram(m),
};
const output = swc.transformSync(code, finalOpts);
const ranges = rangeCapturer.ranges.map(([start, end]) => {
const startLineColumn = rangeByIdxToLineColumn(code, start);
const endLineColumn = rangeByIdxToLineColumn(code, end);
return [...startLineColumn, ...endLineColumn];
})
console.log('ranges', ranges);
return {
map: output.map,
code: output.code,
ranges
};
} |
I believe the problem is related to known behavior in SWC where it seems to append to Trying to work out how to fix it and then I think everything will be working well. |
Thanks! Will try it out tomorrow. I'm still not clear on what ranges are though even after reading the docs, and how they are used for mapping. I assumed they were just something for code coverage. Are they suppose to represent statements or expressions or...? How do they relate to source maps? Will prob need to write in Rust for efficiency. The SWC JS plugin system is being deprecated in the future. |
I can't get it working in JavaScript-land. It looks like once the native binary for
JavaScript source-maps are lossy which means that if your code is transpiled before Wallaby runs, we can't rely on them to identify how the transformed code maps to your original code (it provides series of original start line, start column to transformed start line, start column). Each The You could possibly create a substitute for ranges using source maps and some heuristics but I think there will be a number of errors and weird behaviors because of the lossy nature of source maps. It's normal for JavaScript parsers to include start/end positions for nodes in the AST (e.g. https://astexplorer.net) but in this case, |
The only other thing I can think that might work is to spawn a new process each time you need to compile using swc. On my machine this takes about 40ms per file. Wallaby will cache the files until they are changed so perhaps it's not a bad option if you really need/want |
From the wallaby compiler docs:
So if I understand correctly, the ranges are a depth-first search of the AST? How does Wallaby know what is the format of my "unmodified AST"? What if SWC uses a different AST than Wallaby? Won't that confuse Wallaby? For example using the How does it know to ignore the Should each range just be an execution path rather than nodes?
|
So I am attempting to do this with the source maps, const consumer = await new SourceMapConsumer(rawSourceMap)
const ranges = []
const spans = consumer.eachMapping(m => {
console.log(m)
const {originalLine, originalColumn} = m
ranges.push([originalLine, originalColumn])
})
But I only get the I guess the source map has no knowledge of the AST. So essentially, I would have to parse my original source to an AST (but which one?), but then the speed gains of swc parsing are lost at this point, and I would only benefit from the speed gains from transformation. I think what I might try is adding an attribute to the source maps that swc generates called {
"version":3,
"sources":["repos/vjpr/packages/obs/cli/src/fix-monitors/render-to-html.ts"],
"names":["renderToHtml"],
"mappings":"8BAA8B,YAAY,GAAG,CAAC;AAI9C,CAAC",
+ "ranges: [[x,x,x,x], [x,x,x,x]]
}
|
Yes - this is how Wallaby processes the ranges; I do not believe that the order matters though.
Wallaby doesn't know this and nor does it need to. Wallaby actually parses the
This logic is part of Wallaby's heuristics when processing the
You would have to come up with some heuristics on how to process this... I'm not entirely sure what to suggest, perhaps you could map back to original source and try and determine where the token ends. Perhaps you could also sort the generated to original mappings and come up with another way to automatically generate the end position. Either way, it's not going to be perfect and will result in some weird behaviors.
Correct. I'm not sure what this performance looks like.
I'm guessing you will need to fork I also don't imagine that the Depending on what you are trying to do and what testing framework you want to use, there's another option for you to consider. We (the Wallaby team) actually use Jest with You may be interested to read this blog post: https://wallabyjs.com/blog/optimizing-typescript.html TL;DR: Medium-size project (22,000 LOC, 135 files)
For this project, Wallaby test execution time (approximately Without understanding how you want to set up your project, I would consider opting for something a little more standard (e.g. jest with |
^^^ Haven't read your previous post, will respond soon. I have opened a PR with SWC to traverse the AST and return I also found the bug that incremented the index, which I have noted to the maintainer here: swc-project/swc#1366 (comment). Here is the visitor code I am using: https://github.com/swc-project/swc/pull/2320/files#diff-b1a35a68f14e696205874893c07fd24fdb88882b47c23cc0e0c80a30c7d53759R1116-R1138 Maybe you can suggest how this visitor should look? Should I just visit every expression? Or every node? How do I know its working? I just see this error which I think means the ranges are not correct:
...reading your response now. |
I think you should visit every node; if you just do expressions you'll find some parts of Wallaby don't work properly anymore. For example, if using the debugger, you can select a function parameter and Wallaby will output its value. If you were only outputting expressions, this wouldn't work. The example I provided yesterday (using all visit functions) seemed to work until the subsequent execution because of |
Interesting. I had a feeling there was some magic going on there. So it should be enough to just provide an unordered array of ranges of every node? Perhaps I should use the
The issue I have is I am using pnpm and it doesn't play well with |
I think it would be ideal to provide in the same way that wallaby is (depth-first tree traversal of AST) but I've just gone through our code base and it doesn't seem like this matters. If you run into problems, we would be happy to investigate for you.
Yes - this what I had originally thought the JS code would do yesterday but the start/end changes on subsequent executions as you know...
Fair enough... I'm not familiar enough with pnpm to help. I'm interested to see what the |
I've fixed this here. |
Hmmm... just thinking of another work-around. The position seems offset by the source length of previous transforms so you should be able to track this and account for it over time. I'm going to have a quick play in the repo that you provided yesterday and see if I can get that working. |
I think the idea is that every tool would be re-written in Rust. Allowing JS code to traverse the AST would just slow it down and add too much maintenance. Rust is very tricky for newcomers to write plugins so we will see if this works. I am interested to follow Rome which is now doing their own Rust-based parser/compiler/linter. |
Yep, exactly. I thought of the same, but the API is being deprecated, and it would be slow. But I guess before I can get a PR merged it would be a good workaround. |
OK, so I got it working with the code below. The important part is this: function compile(filename, code, opts) {
const defaultSourceMap = true;
//const defaultSourceMap = 'inline' // originally
const rangeCapturer = new RangeCapturer();
+ let spanStart;
const finalOpts = {
...opts,
sourceMaps:
opts.sourceMaps === undefined ? defaultSourceMap : opts.sourceMaps,
plugin: (m) => {
+ spanStart = m.span.start;
return rangeCapturer.visitProgram(m)
},
};
const output = swc.transformSync(code, finalOpts);
const ranges = rangeCapturer.ranges.map(([start, end]) => {
const startLineColumn = rangeByIdxToLineColumn(code, start - spanStart);
const endLineColumn = rangeByIdxToLineColumn(code, end - spanStart);
return [...startLineColumn, ...endLineColumn];
})
return {
map: output.map,
code: output.code,
ranges
};
} Again, in my code, the idx to line/column mapping is brute force for now but you can optimise that if you want to go with this approach. It'll let you use SWC as it exists right now. I'll close the ticket out for now as I think this gets everything working but obviously you may like to vary your approach. packages/swc-compiler/index.js import * as swc from "@swc/core";
import { Visitor } from "@swc/core/Visitor.js";
class RangeCapturer extends Visitor {
getAllFuncs(toCheck) {
const props = [];
let obj = toCheck;
do {
props.push(...Object.getOwnPropertyNames(obj));
} while ((obj = Object.getPrototypeOf(obj)));
return props.sort().filter((e, i, arr) => {
if (e != arr[i + 1] && typeof toCheck[e] == "function") return true;
});
}
constructor() {
super();
this.ranges = [];
this.getAllFuncs(this).forEach(funcName => {
if (funcName.startsWith('visit')) {
const oldVisit = this[funcName];
this[funcName] = function () {
if (arguments.length > 0) {
if (arguments[0] && arguments[0].span) {
this.ranges.push([arguments[0].span.start, arguments[0].span.end]);
}
}
return oldVisit.apply(this, arguments);
};
}
});
}
}
import { dirname } from "path";
////////////////////////////////////////////////////////////////////////////////
const swcConfig = {
test: ".tsx?$",
sourceMaps: true,
jsc: {
target: "es2020",
parser: {
syntax: "typescript",
tsx: true,
decorators: true,
dynamicImport: true,
},
baseUrl: "./src",
paths: {
//'@src/*': ['*'],
"@src/*": ["./src/*"],
"#src/*": ["./src/*"],
"#packages/*": ["./packages/*"],
},
},
module: {
//type: 'commonjs',
type: "es6",
},
};
////////////////////////////////////////////////////////////////////////////////
function rangeByIdxToLineColumn(source, idx) {
const content = source.substring(0, idx).replace(/\r\n/g, '\n').split('\n');
const line = content.length;
const column = content[content.length - 1].length;
return [line, column];
}
export default function (w, { shouldLog } = {}) {
return (file) => {
const { type, path, content, config } = file;
const filename = path;
if (shouldLog) console.log("Compiling:", filename);
const plugins = [];
const opts = {
sourceRoot: dirname(filename),
filename,
//jsc // Config options can be added here.
jsc: { ...swcConfig.jsc },
//plugin: swc.plugins(plugins),
};
const res = compile(filename, content, opts);
//console.log({filename, content, res})
return res;
};
}
////////////////////////////////////////////////////////////////////////////////
function compile(filename, code, opts) {
const defaultSourceMap = true;
//const defaultSourceMap = 'inline' // originally
const rangeCapturer = new RangeCapturer();
let spanStart;
const finalOpts = {
...opts,
sourceMaps:
opts.sourceMaps === undefined ? defaultSourceMap : opts.sourceMaps,
plugin: (m) => {
spanStart = m.span.start;
return rangeCapturer.visitProgram(m)
},
};
const output = swc.transformSync(code, finalOpts);
const ranges = rangeCapturer.ranges.map(([start, end]) => {
const startLineColumn = rangeByIdxToLineColumn(code, start - spanStart);
const endLineColumn = rangeByIdxToLineColumn(code, end - spanStart);
return [...startLineColumn, ...endLineColumn];
})
return {
map: output.map,
code: output.code,
ranges
};
} |
Great, thanks for you help! |
Did you test this in IntelliJ? I'm only seeing inline annotations at the top of the file. For this file my ranges look like:
Same behavior with your JS visitor, as well as Rust visitor. |
Maybe it's that the source maps are wrong because of that bug... |
I didn't but it won't make a difference. You may need to force your Wallaby cache to reset. Try changing your |
Ok the issue seems to be that my source maps coming from swc were messed up. Everything was on line 1. Must have been something from my hacking on |
Would it be possible to share the I would prefer all my code to only be transformed with SWC, and was thinking of just parsing with Babel or ESBuild to generate the ranges. Getting ranges from SWC will not be possible until their Rust plugin API is ready. UPDATE: Wrote my own babel compiler because I needed async ESM configuration (#2821). Just used Hope this works...although maybe there are some edge-cases your impl covers. The benefit is that I can use this to parse the AST and generate ranges while using SWC as the transformer. import {transformFileAsync, parseAsync} from '@babel/core'
import traverse from '@babel/traverse'
const ast = await parseAsync(src, opts)
function getRangesFromBabelAst(ast) {
let ranges = []
traverse(ast, {
enter(path) {
//console.log(path.node)
const loc = path.node.loc
if (!loc) return
ranges.push([
loc.start.line,
loc.start.column,
loc.end.line,
loc.end.column,
])
},
})
return ranges
} |
Also a question: From https://wallabyjs.com/blog/optimizing-typescript.html:
Whenever I start Wallaby it starts a ton of processes. Is this the "processor pool" that is mentioned? Do these not compile files in parallel? What are these processors doing? Is there a way to limit them too...it usually starts 10+
|
This is the case by default, unless you are using
With a custom compiler, yes, these are compiling in parallel.
We do not currently have a setting for this. The number of processes used is equal to Are you seeing something different? E.g. high CPU on every start? If it is behaving as expected (i.e. is only really a first time starting on project or after config change), I'm assuming that you've noticed the problem because you've been writing a compiler and it's probably OK to leave as is. If not, please let us know. |
Nope. It's working as expected. |
Issue description or question
I have a simple test that looks like this in
<projectCacheDir>/original
:On initial run, in
<projectCacheDir/instrumented
I get the following which works as expected:But then when I make a change and re-run the tests,
import 'ava'
is converted to a require:Only
ava
has this behavior. If I rename it toava1
, then it's an import.This causes:
What could be causing this?
Does wallaby do any import rewriting?
I also tried wrapping ava in
my-ava
, it didn't rewrite anymore, but now I do not get any inline console messages printed in IntelliJ.Is ava4 + wallaby working with native es modules? Or do I need to transpile them to CJS?
Also, when compiling with SWC, are
ranges
really necessary? Would this break IntelliJ inline messages?The text was updated successfully, but these errors were encountered: