refactor(core): use # instead __ as internal handler prefix #1104
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. |
relu91
approved these changes
Oct 2, 2023
Member
relu91
left a comment
relu91
left a comment
There was a problem hiding this comment.
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.
danielpeintner
approved these changes
Oct 3, 2023
Member
danielpeintner
left a comment
danielpeintner
left a comment
There was a problem hiding this comment.
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..
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 theExposedThingclass 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.