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

feat(neon-macros): Convert snake_case to camelCase when exporting functions #1084

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

kjvalencik
Copy link
Member

Rust uses a snake_case convention for functions while JavaScript camelCase. This changes makes the default behavior of neon::export convert to camelCase to follow JavaScript naming conventions.

If the user does not want this behavior the name = ".." override may be used to explicitly provide a name.

No mapping is provided for const or static exported globals since both Rust and JS follow a YELLING_SNAKE_CASE convention.

@kjvalencik kjvalencik requested a review from dherman November 22, 2024 17:57
Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

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

I like this, I think it's a better default. For the implementation, I'm proposing that we use convert_case. See the comments inline for my rationale.

crates/neon-macros/src/export/function/mod.rs Outdated Show resolved Hide resolved
crates/neon-macros/src/export/function/mod.rs Show resolved Hide resolved
@kjvalencik kjvalencik force-pushed the kv/camel-case branch 4 times, most recently from f152cd3 to 687aadb Compare November 25, 2024 17:06
@dherman
Copy link
Collaborator

dherman commented Nov 25, 2024

We discussed this on Slack and came to the following conclusions:

  • convert_case doesn't handle things like leading and trailing underscores
  • Using an external crate is a bit risky since this is part of the public behavior of Neon
  • Even using regex would be a bit complex so it ends up being fine to implement this by hand
  • We want to be pretty conservative about what conventions we recognize; if they have nonstandard names we will just bail and not transform the name

In the end this means:

  • Preserve leading and trailing underscores
  • Only capitalize the first character after a non-leading underscore
  • Bail if we see two underscores in a row in between words or a capital letter

Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

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

This looks great. Ship it!

@kjvalencik kjvalencik merged commit cb8840a into main Nov 26, 2024
9 checks passed
@kjvalencik kjvalencik deleted the kv/camel-case branch November 26, 2024 17:01
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.

2 participants