refactor(trie): store opts in private property with defaults
#2224
Conversation
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Signed-off-by: Brian Faust <[email protected]>
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
@holgerd77 conflicts resolved |
There was a problem hiding this comment.
Oh this is a beauty, wonder if we want to last-minute do for the other libraries too? 🤔 (some substantial amount of work though, so at least 1 person, 1 day, but might be worth it, eventually we can also still be doing between RC and final but before would likely be better).
Anyhow, so would give an approval here as well (not formally yet).
packages/trie/src/trie/trie.ts
Outdated
| @@ -35,38 +35,39 @@ interface Path { | |||
| * The API for the base and the secure interface are about the same. | |||
| */ | |||
| export class Trie { | |||
| #opts: TrieOptsWithDefaults = { | |||
There was a problem hiding this comment.
What is this # standing for respectively doing? 🤔
There was a problem hiding this comment.
There was a problem hiding this comment.
The TLDR is a real private property, not just a visibility modifier like private which has no effect at runtime. Visibility modifiers other than # don't actually make those properties inaccessible once the code is turned into JS.
There was a problem hiding this comment.
Ah, just googled a bit, this is an ES2022 property which is beyond our build target, let's please not introduce any new language constructs here last minute.
This property is super useful, but not anything any more for this round of breaking releases. I can very well imagine a broad set of refactoring around this along the next breaking release round. 🙂
Signed-off-by: Brian Faust <[email protected]>
|
So I could take this in for inclusion as well. |
Instead of having separate properties for everything we can use a default object, merge properties and then access everything through that object.