Skip to content

fix(cli): use jsPackageName from cli options#3035

Merged
Brooooooklyn merged 1 commit intonapi-rs:mainfrom
U-C-S:fix-js-pkg-name
Nov 27, 2025
Merged

fix(cli): use jsPackageName from cli options#3035
Brooooooklyn merged 1 commit intonapi-rs:mainfrom
U-C-S:fix-js-pkg-name

Conversation

@U-C-S
Copy link
Contributor

@U-C-S U-C-S commented Nov 26, 2025 •
edited
Loading

After upgrading to v3, even after setting the CLI option --js-package-name, napi cli still generates the bindings.js using the npm package name.

This fix makes sure the bindings.js are generated using the jsPackageName if a value exists, else uses npm package name as previously.

@graphite-app
Copy link

graphite-app bot commented Nov 26, 2025

How to use the Graphite Merge Queue

Add the label ready-to-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@coderabbitai
Copy link

coderabbitai bot commented Nov 26, 2025 •
edited
Loading

Walkthrough

The writeJsBinding invocation in the build API now conditionally uses an optional jsPackageName override from options before falling back to the standard config package name, enabling flexibility in naming generated JavaScript bindings.

Changes

Cohort / File(s) Summary
JS Binding Configuration
cli/src/api/build.ts		 Modified writeJsBinding invocation to use this.options.jsPackageName when defined, otherwise defaults to this.config.packageName. Introduces optional npm package name override for generated JS bindings.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file modified with a straightforward conditional parameter override
  • No new logic, error handling, or control flow changes
  • Low risk of unintended side effects given the fallback pattern

Poem

🐰 A package name, once carved in stone,
Now flexes with a gentle moan,
Options override, or defaults remain—
The binding flows with freedom's reign! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: using jsPackageName from CLI options instead of directly from config, which matches the core modification in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot reviewed Nov 26, 2025
View reviewed changes
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
cli/src/api/build.ts (1)

910-921: CLI override wiring for packageName looks correct

Using this.options.jsPackageName ?? this.config.packageName cleanly adds the CLI override while preserving existing behavior when the flag is unset. No type or runtime concerns here.

If the intention is for jsPackageName to be the canonical JS package identifier across all generated bindings (including WASI/browser ones that still use this.config.packageName), you may want to consider applying the same override there as well; otherwise this change is consistent as-is.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc4d475 and b113aaf.

📒 Files selected for processing (1)
  • cli/src/api/build.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: CR
Repo: napi-rs/napi-rs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:31:23.866Z
Learning: Applies to /crates/**/*.rs : Use `#[napi(js_name = "...")]` attribute to rename Rust items in JavaScript/TypeScript

Learnt from: CR
Repo: napi-rs/napi-rs PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:31:23.866Z
Learning: Applies to /crates/**/*.rs : Use `#[napi]` attribute on impl blocks to define classes exported to JavaScript/TypeScript

@Brooooooklyn Brooooooklyn merged commit 94a6d37 into napi-rs:main Nov 27, 2025
74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@U-C-S @Brooooooklyn