Skip to content

fix(codemod-sandbox): Replace row with line in JsPosition to match the types from @ast-grep/napi #1655

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 27, 2025
Merged

fix(codemod-sandbox): Replace row with line in JsPosition to match the types from @ast-grep/napi #1655

merged 1 commit into from
Jul 27, 2025

Conversation

brunocroh
Copy link
Contributor

@brunocroh brunocroh commented Jul 27, 2025 •
edited
Loading

📚 Description

The type Position in @codemod.com/jssg-types differs from what is returned in Range objects in JavaScript.

Looking at the ast-grep Rust implementation, the line is returned as row there. If this change needs to be made in the Rust code rather than in the type definitions, please let me know, I’d be happy to contribute to the Rust code (I tried to run it but had some issues setting up the development environment, so I’ll need some help with it. I can also use this opportunity to document how to setup development environment in the CONTRIBUTING.md).

codemod/crates/codemod-sandbox/src/ast_grep/sg_node.rs

Lines 154 to 165 in 6578c16

let result = JsNodeRange {
start: crate::ast_grep::types::JsPosition {
row: start_pos_obj.line(),
column: start_pos_obj.column(&self.inner_node),
index: byte_range.start,
},
end: crate::ast_grep::types::JsPosition {
row: end_pos_obj.line(),
column: end_pos_obj.column(&self.inner_node),
index: byte_range.end,
},
};

🔗 Linked Issue

nodejs/userland-migrations#136

@vercel Vercel
Copy link

vercel bot commented Jul 27, 2025

@brunocroh is attempting to deploy a commit to the Codemod Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link

CLAassistant commented Jul 27, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@brunocroh brunocroh mentioned this pull request Jul 27, 2025
Merged
@mohebifar
Copy link
Member

Thanks for pointing that out! Good catch. I believe the change should be made in the Rust code instead. Let me know if you’d like to update the PR or if you need any assistance.

@brunocroh brunocroh changed the title fix(jssg-types): replace line with row in Position type fix(codemod-sandbox): Replace row with line in JsPosition to match the types from @ast-grep/napi Jul 27, 2025
@brunocroh
Copy link
Contributor Author

brunocroh commented Jul 27, 2025 •
edited
Loading

Thanks for pointing that out! Good catch. I believe the change should be made in the Rust code instead. Let me know if you’d like to update the PR or if you need any assistance.

Okay, so I dropped the previous commit and pushed a new one. Now I'm updating the JsPosition object to replace row with line.
If I understand the code correctly, this is the bridge between wasm and Rust, so I'm just updating the names on the bridge/JavaScript side, the sandbox logic remains unchanged.

Let me know if anything else needs to be done, thank you.

@mohebifar
Copy link
Member

Thank you! I tested these changes and it all looks good. Waiting for CI 👀

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Jul 27, 2025

Open in StackBlitz

npm i https://pkg.pr.new/codemod@1655

commit: e5a77fd

@mohebifar mohebifar merged commit ac1468a into codemod-com:main Jul 27, 2025
8 of 10 checks passed
@mohebifar
Copy link
Member

Thank you so much! These changes will be included in the next release which would be 1.0.0-rc.19

@brunocroh
Copy link
Contributor Author

Brilliant, thank you mate!

This was referenced Jul 27, 2025
Open
Open
@mohab-sameh
Copy link
Contributor

@all-contributors add @brunocroh for code

@allcontributors allcontributors bot mentioned this pull request Jul 28, 2025
Merged
@allcontributors AllContributors
Copy link
Contributor

@mohab-sameh

I've put up a pull request to add @brunocroh! 🎉

@mohab-sameh
Copy link
Contributor

I can also use this opportunity to document how to setup development environment in the CONTRIBUTING.md)

If you'd like to open a PR and I'll take care of any polishes required, that would be amazing. Thanks, @brunocroh!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mohebifar mohebifar mohebifar approved these changes

Assignees
No one assigned
Labels
❤️ community contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@brunocroh @CLAassistant @mohebifar @mohab-sameh