-
Notifications
You must be signed in to change notification settings - Fork 16
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
Probes branch: same nodeId is given to the different initializers in "var b = 2, c = 3" #11
Comments
Oh, I missed this before (my e-mail inbox is a mess!). That's great! Debugging d3 was one of the primary motivations for the probes branch... As of 4a731a9 all probes have unique identifiers and report the actual location of the expression. But it doesn't yet generate all the probes you'd want for chained calls like this: d3.select("foo") // no probe
.selectAll("blah") // no probe
.css("a", "b"); // probe How granular do you need the probes to be for what you're working on? |
Right, the probes by themselves wouldn't let you inspect into a method chain. But since fondue also traces the function calls, I think that the information I would need would be available if I could inspect the traced function and the arguments from inside But notice I'm making a big assumption here, which is that there would be some way I could hook into the client-side internals of fondue. This is how I was designing my But I'd like to be able to put es-spy on the back burner and focus on developing a UI a d3-specific code instead. For the d3 stuff, I would want to do a fair amount of interpretation right in the same context as the d3 code being inspected. This is because d3 is so declarative; just looking at the return values and arguments in a typical d3 method chain isn't very enlightening because the method chain is just setting up relationships. The actual, concrete drawing operations are sort of hidden away. Instead I want the user to be able to click on a graphical element, say an SVG In order to do that, I need to be able to annotate each d3 object I see flying around with some additional information: these DOM elements were added during the execution of This is getting a bit out of scope for this particular Github issue, though; perhaps I should write to the Theseus mailing list? |
Regarding the particular line of code mentioned in the issue title, I created PR #13. Still reporting an issue rather than fixing it, but I'm getting up to speed. |
Got it! I'd be willing to move to the mailing list if you like. We could also keep hijacking this issue, or you could make a new issue! I prefer GitHub because it's easy to embed code and links to commits. For the provenance features you're describing, it seems like all you'll need is:
Of course, you've thought about what you want to do much more than I have, so this could be wrong. But it seems like a reasonable way to proceed is for you to hack those into your copy of fondue until it's clear that that's really all you need. At that point, I'd be willing to either integrate your changes or provide the extension points you need to make your stuff work without modification of fondue. You might be interested in the simplify branch I just pushed, where I'm starting to modularize fondue to make it easier to understand and extend. For example, I experimentally modified fondue to print the result of every expression by adding this block (and removing the "callsite" block so I didn't have to be careful about their interaction): {
selector: 'expression',
callback: function (node) {
var id = makeId('expression', options.path, node.loc);
node.wrap(options.tracer_name + '.traceValue(' + JSON.stringify(id) + ', ', ')');
},
}, Then when I evaluated something like var a = 4;
var b = 2;
var c = a*a + b*b;
console.log('the answer is: ' + c); I get output like:
So if the way I chose to implement probes doesn't actually seem to work for your use case, it might be easier to add just the parts you need to the "simplify" branch. Unfortunately, it's not nearly as well-tested as master, and esprima-selector is intentionally brittle because I didn't want to bother enumerating every possible AST node type before I had an idea of whether the API was any good. But I intend to eventually make this master and it currently passes all the tests. |
Quickly, re the simplify branch and esprima-selector, have you seen https://github.com/jrfeenst/esquery? Your expressions example challenges me to remember why the 80/20 solution of just logging the expressions wasn't "good enough" for es-spy. (Well, of course you don't want to inadvertently evaluate left-hand-side expressions, and I originally wanted to evaluate the Identifier Anyway, I'll take a look at the simplify branch shortly, and try to move quickly (with a perfectly nice day job, "quickly" is relative) to a proof of concept that shows to myself that I can pull out the d3-specific information I need. If I get that to work, I'd lean towards developing hooks so that I can build on top of Fondue rather than putting d3-specific code into Fondue itself. |
I hadn't seen that! I'd surfed npm for at least an hour to find something to use and somehow it didn't come up. Thanks for the link! I'll see if it's appropriate. One of the requirements is that it works inside falafel's callback so that rewrites happen in the correct order. It doesn't seem to work that way from skimming the tests, but I've only had time to skim. Cool. Hope to hear more from you as things progress. |
Just a heads up, I was checking out the probes branch and noticed one assertion in
test/test-vals.js
was failing.While debugging I noticed that the same
nodeId
(scripts/vals.js-probe-2-17-2-17
) is applied to the init nodes of the two different VariableDeclarators in the VariableDeclarationvar b = 2, c = 3
found intest/scripts/vals.js
.I assume this would cause all downstream code to be confused about the distinction between the two initializers.
PS. I'm working on a retrospective debugger for d3 "blocks", and would love to leverage fondue or contribute to it rather than continue to work on my own instrumentation library.
The text was updated successfully, but these errors were encountered: