-
Notifications
You must be signed in to change notification settings - Fork 59
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
Map Proxy API + Implementation #92
Conversation
da42b3f
to
eeae621
Compare
…a Data object is passed to it
…wCodec custom codec
cf7131d
to
ec0d05b
Compare
8dd3062
to
bd75e99
Compare
Test PASSed. |
var partitionsToKeys: {[id: string]: any} = {}; | ||
var key: K; | ||
for (var i in keys) { | ||
key = keys[i]; |
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.
Why declare outside of the loop and then reassign within it?
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.
Putting it inside for method does not make its scope restricted to for loop. So I intentionally put them outside. This way it is less likely that someone mistakenly introduces the same variable later in the same method expecting it to be null. Matter of taste.
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.
If you use let variableName = value it will be restricted to the loop scope.
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.
Since we use Typescript compiler target as ES5
all let
s are transpiled to var
. So this would be worse. You expect it to be block scoped and get function scoped. Since Node 4 do not support majority of ES6 features we have to stick with ES5 and var
s.
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.
OK
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.
There is requests from community to make Typescript support selectively turning on and off certain features: microsoft/TypeScript#4389 . I hope they support this feature and we can use cool ES6 features that Node already supports.
Test PASSed. |
Any idea when this will be published to npm? |
Hello @jackspaniel . This is already merged to master so it will be available with |
fixes #18. Note that this API does not include
executeOnKeys
,addInterceptor
and Predicates related methods. These will come with Serialization implementation.