Skip to content

Add a way to add setters/getters in js_class! #3911

Merged
raskad merged 6 commits intoboa-dev:mainfrom
hansl:getter_setter
Jul 21, 2024
Merged

Add a way to add setters/getters in js_class! #3911
raskad merged 6 commits intoboa-dev:mainfrom
hansl:getter_setter

Conversation

@hansl
Copy link
Contributor

@hansl hansl commented Jul 10, 2024

And update documentation for more explanation around the macro.

@hansl hansl marked this pull request as draft July 10, 2024 23:44
@hansl
Copy link
Contributor Author

hansl commented Jul 10, 2024

Converting to draft as I've come up with a better interface for getter/setter. Basically:

prop name_of_prop as "nameOfProp" {
  fn getter(...) {}
  fn setter(...) {}
}

with getter and setter optional. That way we don't need 3 sections for properties and it's a bit simpler.

@codecov
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 51.59%. Comparing base (6ddc2b4) to head (2a4aa1b).
Report is 211 commits behind head on main.

Files Patch % Lines
core/interop/src/macros.rs 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3911      +/-   ##
==========================================
+ Coverage   47.24%   51.59%   +4.35%     
==========================================
  Files         476      468       -8     
  Lines       46892    44839    -2053     
==========================================
+ Hits        22154    23136     +982     
+ Misses      24738    21703    -3035     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hansl
Copy link
Contributor Author

hansl commented Jul 11, 2024

Okay this does use the new API now.

@hansl hansl marked this pull request as ready for review July 11, 2024 00:30
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Good work! Looks perfect to me.

@jedel1043 jedel1043 requested a review from a team July 17, 2024 01:18
@jedel1043 jedel1043 added A-Enhancement New feature or request A-API Changes related to public APIs labels Jul 17, 2024
@jedel1043 jedel1043 added this to the next-release milestone Jul 17, 2024
@jedel1043
Copy link
Member

jedel1043 commented Jul 17, 2024

Thought: the syntax is getting a bit too DSL-like, so I'll probably see if I can start working on that proc macro.

@raskad raskad added this pull request to the merge queue Jul 21, 2024
Merged via the queue into boa-dev:main with commit 816e1bb Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-API Changes related to public APIs A-Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants