-
Notifications
You must be signed in to change notification settings - Fork 80
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
refactor(core): use # instead __ as internal handler prefix #1104
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1104 +/- ##
==========================================
+ Coverage 75.52% 75.54% +0.01%
==========================================
Files 80 80
Lines 16153 16164 +11
Branches 1520 1513 -7
==========================================
+ Hits 12200 12211 +11
Misses 3914 3914
Partials 39 39
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks good. I would say that those fields could be better modeled as protected (hence the private modifier would not work), but let's see if we find a use case for extending the exposed-thing in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM and thanks @JKRhb
I just added the comment that it might make sense to put a statements why the # prefix has been used for the internal handlers..
In the context of #1097, I remembered that JavaScript nowadays has a built-in prefix for private methods and class fields (
#
).As it turns out, fields with this prefix get ignored when calling
JSON.stringify
, so using it instead of__
for the internal handler maps of theExposedThing
class enables us to get rid of some checks during serialization, which should make the code a bit more idiomatic and slightly increase its performance.As the handler maps are now truly private, I had adjust some of the test code for it to still work. However, I think the code in question was rather trivial, so this change should hopefully be fine.