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

Codemods: add support for codemod debugging #19703

Merged
merged 1 commit into from
Nov 15, 2017
Merged

Conversation

jsnajdr
Copy link
Member

@jsnajdr jsnajdr commented Nov 13, 2017

I wanted to debug a codemod script today and found it's not easy. Node needs to be run with --inspect and --debug-brk parameters. That spawns a debugger server and breaks on first statement. jscodeshift must be prevented from spawning worker processes by running it with --run-in-band option. Otherwise, the transform will run in a process different from the one we are debugging.

This PR adds a --debugger option to the run script.

How to test:

  1. Run npm run codemod -- --debugger transform-name target-name
  2. Open the chrome-devtools:// link you see in terminal in Chrome
  3. Enjoy the Chrome debugger paused on the first statement -- set breakpoints, put debugger statements in your code... and run.

I don't know how Node debugging works in editors like VS Code. Maybe @blowery could help with making the debugging work in these?

Some links where I found valuable advice:

@jsnajdr jsnajdr added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 13, 2017
@jsnajdr jsnajdr self-assigned this Nov 13, 2017
@matticbot
Copy link
Contributor

@simison
Copy link
Member

simison commented Nov 13, 2017

Awesome! Looking forward to use this. :-)

I tested this with Node.js 6.1.5 and Node 8.7.0 (because #19317).

Works great on Node 6, just like you described. Also remains working without the debug param.

On Node 8 this works fine too, I just get a deprecation warning:

$ node --version
v8.7.0

$ npm run codemod -- --debugger single-tree-rendering ./client

> wp-calypso@0.17.0 codemod /####/wp-calypso
> node bin/codemods/run "--debugger" "single-tree-rendering" "./client"


Running single-tree-rendering on ./client
Debugger listening on ws://127.0.0.1:9229/cabe5ade-e762-4944-a41f-96de1cefbd5e
For help see https://nodejs.org/en/docs/inspector
Debugger attached.
(node:19971) [DEP0062] DeprecationWarning: `node --inspect --debug-brk` is deprecated. Please use `node --inspect-brk` instead.

@simison simison requested a review from ehg November 13, 2017 15:20
@simison
Copy link
Member

simison commented Nov 13, 2017

As a sidenote, I looked into adding support for jscodeshift's --ignore-pattern, but found it a bit complicated with current setup. Adding something like yargs as a parser would probably simplify this if we want to add any other features later on, but I don't think it's worth adding in just yet. It's also nice not to have one more dependency. :-)

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Very nice, and thanks for the documentation 😀

I left a few documentation suggestions mostly for simplicity and brevity. It's to you whether you want to make any of the changes.

If you are a codemod author, you may want to debug your codemod using the Chrome debugger. Then
run the codemod script with a `--debugger` parameter:
```bash
./bin/codemods/run.js --debugger my-transform client/target.js
Copy link
Member

Choose a reason for hiding this comment

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

I think for the docs we can omit this and just include a canonical npm run codemod …. The savvy can figure out the direct invocation if they're interested.

```
This will run `jscodeshift` in a Node process that has command line arguments `--inspect` and
`--debug-brk`. That means that a debugger will be attached to the Node process and will break on
the first statement. That allows you to connect with Chrome and run the codemod script.
Copy link
Member

Choose a reason for hiding this comment

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

It sounds like we may be changing the node args soon. How about leaving off the specifics:

This will run jscodeshift in a Node process with the appropriate flags for debugging. The process will break on the first debugger statement, allowing inspection with Chrome.

the first statement. That allows you to connect with Chrome and run the codemod script.

`jscodeshift` will also be run with a `--run-in-band` option, telling `jscodeshift` not to spawn
worker processes and run the transformation inside one Node process -- the one being debugged.
Copy link
Member

Choose a reason for hiding this comment

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

If we decide to update the other parts, we might go for a higher level description here as well:

The codemod will be run in a single process to ensure that the debugger is captured, as opposed to normal invocation which spawns worker processes.

@jsnajdr jsnajdr force-pushed the codemod/support-debugging branch from 25c285f to 41eb147 Compare November 15, 2017 12:23
@jsnajdr
Copy link
Member Author

jsnajdr commented Nov 15, 2017

Hello @sirreal! i updated the documentation according to your suggestions. I keep mentioning the low-level command line args, but deemphasized into a parenthesized remark.

I also updated the cmdline args to Node 8.

Please have one last look at the changes before I merge them.

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Nice, I just tested on with node 8.

:shipit:

@sirreal sirreal added [Status] Ready to Merge [Type] Enhancement and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 15, 2017
@jsnajdr jsnajdr merged commit b0f3a66 into master Nov 15, 2017
@jsnajdr jsnajdr deleted the codemod/support-debugging branch November 15, 2017 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants