Skip to content

Conversation

@xusd320
Copy link
Contributor

@xusd320 xusd320 commented Dec 18, 2025

This pull request makes several improvements and cleanups to the DOM parser's Rust and JavaScript bindings. The main changes include refining attribute handling in Rust, removing some unnecessary JavaScript exports, and updating the expected version for native bindings in the Node.js loader. Additionally, several N-API annotations have been removed from Rust methods, likely to streamline or refactor the JS interface.

Rust binding improvements and cleanup:

  • Improved the get_attribute method in NodeRepr to better handle attribute lookup, including a fallback to search by qualified name, and updated the logic for retrieving attribute names to include namespace prefixes when present. [1] [2]
  • Removed the get_children and get_attributes methods from NodeRepr, streamlining the Rust API.
  • Removed unnecessary #[napi] and #[napi(js_name = ...)] annotations from several Rust methods, possibly as part of a refactor to manage the JS interface differently. [1] [2] [3] [4] [5] [6]

JavaScript/Node.js binding updates:

  • Updated the expected version for all native bindings from 0.0.0 to 0.0.1 in the version check logic of index.js, ensuring compatibility with the new native release. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]
  • Removed the export of the parse function from domparser.js, likely to simplify the public API.

These changes collectively improve the maintainability and correctness of the bindings, and align the JS and Rust interfaces more closely.

@gemini-code-assist
Copy link

Summary of Changes

Hello @xusd320, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on refining the API surface of the domparser_napi crate by removing specific methods and their N-API exposures that are deemed non-standard or unnecessary. The changes aim to streamline the interface between Rust and Node.js, ensuring a cleaner and more focused set of functionalities are available, alongside corresponding updates to JavaScript module exports and TypeScript definitions.

Highlights

  • Node.js API Exposure Changes: Several methods within the NodeRepr Rust implementation, including clone_self_only, clone_recursive, select, select_all, outer_html, inner_html, and text, have had their #[napi] attributes removed. This means these methods are no longer directly exposed or accessible from Node.js.
  • Method Removals: The get_children and get_attributes methods have been entirely removed from the NodeRepr struct in Rust, making them unavailable both internally and externally.
  • JavaScript Module Cleanup: The domparser.js module has been updated to remove the direct export of the parse function, now only exporting NodeRepr and DOMParser.
  • TypeScript Definition Updates: The TypeScript declaration files (index.d.ts) have been updated to reflect the removal of the aforementioned methods and the parse function, ensuring type consistency with the updated API.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively refactors the API to align more closely with standard DOM APIs by removing non-standard methods and favoring property-based access (e.g., innerHTML instead of innerHtml()). The changes are consistent across the Rust implementation and the corresponding TypeScript definitions. I've identified a minor issue in the TypeScript definition files where a method is declared twice, which I've commented on. Overall, this is a solid improvement for API consistency and usability.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request removes non-standard API methods from the Node.js bindings, standardizing the API surface to match standard DOM APIs. The changes primarily involve removing the #[napi] attribute from several internal Rust methods and cleaning up JavaScript exports.

Key Changes:

  • Removed non-standard methods (select, selectAll, getChildren, getAttributes, clone, cloneRecursive, outerHtml, innerHtml, text) from the public API while keeping standard DOM equivalents (querySelector, querySelectorAll, cloneNode)
  • Removed parse function export from domparser.js while keeping it available through DOMParser.parseFromString
  • Updated TypeScript definitions to reflect the new API surface

Reviewed changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
index.d.ts Removed type declarations for non-standard methods that are no longer exposed to JavaScript
domparser.js Removed parse from module exports (still available internally for DOMParser.parseFromString)
crates/domparser_napi/src/node_repr/query.rs Removed #[napi] attributes from select, select_all, outer_html, inner_html, and text methods; deleted get_children and get_attributes methods entirely
crates/domparser_napi/src/node_repr/mod.rs Removed #[napi] attributes from clone_self_only and clone_recursive methods (still used internally by cloneNode)
crates/domparser_napi/index.d.ts Removed type declarations matching the changes in the root index.d.ts (auto-generated file)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

@kuitos kuitos left a comment

Choose a reason for hiding this comment

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

LGTM

@xusd320 xusd320 force-pushed the refactor/remove-non-standard-apis branch from fa00806 to e4a1265 Compare December 18, 2025 06:47
@xusd320 xusd320 merged commit f0c8df7 into main Dec 18, 2025
17 checks passed
@xusd320 xusd320 deleted the refactor/remove-non-standard-apis branch December 18, 2025 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants