Skip to content
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.

Sign up for GitHub

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

src: fixup strings, general code cleanup #14937

Closed
wants to merge 2 commits into from
Closed

src: fixup strings, general code cleanup #14937

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9990f11
src: fixup strings, reduce duplication
jasnell Aug 19, 2017
50524da
src: reduce code duplication
jasnell Aug 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
src: reduce code duplication 
Adds `AsyncWrap::AddWrapMethods()` to add common methods
to a `Local<FunctionTemplate>`. Follows same pattern as
stream base.
  • Loading branch information
@jasnell
jasnell committed Aug 19, 2017
commit 50524dac154234a1b28950bd6c5f817d09724ae7
7 changes: 7 additions & 0 deletions src/async-wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,13 @@ void AsyncWrap::QueueDestroyId(const FunctionCallbackInfo<Value>& args) {
PushBackDestroyId(Environment::GetCurrent(args), args[0]->NumberValue());
}

void AsyncWrap::AddWrapMethods(Environment* env,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we bikeshed about the name.
IMHO Add -> Bind and Wrap -> AsyncHooks so BindAsyncHooksMethods

Copy link
Member

Choose a reason for hiding this comment

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

Add -> Bind

Why? I’d disagree.

Wrap -> AsyncHooks

Not the same things.

Copy link
Member

@addaleax addaleax Aug 19, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

AddAsyncWrapMethods might be more explicit, if you like it. Disregard that, this is static method anyway, so it would always seem redundant.

Copy link
Contributor

Choose a reason for hiding this comment

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

I felt AddWrapMethods doesn't convey the semantics clearly, so I tried to decostruct, and see what felt wrong.

Add -> Bind

Why? I’d disagree.

Add has (for me) the connotation of creating something new, and since env->SetProtoMethod AFAICT only creates JS binding for an existing native method Bind sounds more appropriate. But we can also extend the existing metaphor and use Set.

Wrap -> AsyncHooks

Not the same things.

Agreed, but AFAIK getAsyncId and asyncReset are new and only used in the context of the hooks so AsyncHooks felt more focused.
Maybe AsyncIDs is even better.

So to sum another option SetAsyncIDsMethods?

Copy link
Contributor

Choose a reason for hiding this comment

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

On third thought AsyncHooks is not good, although that's the "context", the methods are only used by the hooks, and are not the hooks. so:
+2 for AsyncIDs
+1 for AsyncWrap

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm -1 on bikeshedding on this actually. Just don't see much point in doing so.

Local<FunctionTemplate> constructor,
int flag) {
env->SetProtoMethod(constructor, "getAsyncId", AsyncWrap::GetAsyncId);
if (flag & kFlagHasReset)
env->SetProtoMethod(constructor, "asyncReset", AsyncWrap::AsyncReset);
}

void AsyncWrap::Initialize(Local<Object> target,
Local<Value> unused,
Expand Down
9 changes: 9 additions & 0 deletions src/async-wrap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,22 @@ class AsyncWrap : public BaseObject {
PROVIDERS_LENGTH,
};

enum Flags {
Copy link
Member

Choose a reason for hiding this comment

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

Since you’re passing it as ints, and since we have C++11 available: How about enum class JSMethodFlags { (which would implicitly have the underlying type int)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Wrote it this way to be consistent with StreamBase::Flags. Converting these to enum class ... is a bit more involved than I'd like to get here. If we want to convert those, then we can do both AsyncWrap::Flags and StreamBase::Flags at the same time in a separate PR.

kFlagNone = 0x0,
kFlagHasReset = 0x1
};

AsyncWrap(Environment* env,
v8::Local<v8::Object> object,
ProviderType provider,
bool silent = false);

virtual ~AsyncWrap();

static void AddWrapMethods(Environment* env,
v8::Local<v8::FunctionTemplate> constructor,
int flags = kFlagNone);

static void Initialize(v8::Local<v8::Object> target,
v8::Local<v8::Value> unused,
v8::Local<v8::Context> context);
Expand Down
8 changes: 4 additions & 4 deletions src/cares_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2189,7 +2189,7 @@ void Initialize(Local<Object> target,
Local<FunctionTemplate> aiw =
FunctionTemplate::New(env->isolate(), is_construct_call_callback);
aiw->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(aiw, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, aiw);
Local<String> addrInfoWrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "GetAddrInfoReqWrap");
aiw->SetClassName(addrInfoWrapString);
Expand All @@ -2198,7 +2198,7 @@ void Initialize(Local<Object> target,
Local<FunctionTemplate> niw =
FunctionTemplate::New(env->isolate(), is_construct_call_callback);
niw->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(niw, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, niw);
Local<String> nameInfoWrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "GetNameInfoReqWrap");
niw->SetClassName(nameInfoWrapString);
Expand All @@ -2207,7 +2207,7 @@ void Initialize(Local<Object> target,
Local<FunctionTemplate> qrw =
FunctionTemplate::New(env->isolate(), is_construct_call_callback);
qrw->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(qrw, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, qrw);
Local<String> queryWrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "QueryReqWrap");
qrw->SetClassName(queryWrapString);
Expand All @@ -2216,7 +2216,7 @@ void Initialize(Local<Object> target,
Local<FunctionTemplate> channel_wrap =
env->NewFunctionTemplate(ChannelWrap::New);
channel_wrap->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(channel_wrap, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, channel_wrap);

env->SetProtoMethod(channel_wrap, "queryAny", Query<QueryAnyWrap>);
env->SetProtoMethod(channel_wrap, "queryA", Query<QueryAWrap>);
Expand Down
2 changes: 1 addition & 1 deletion src/fs_event_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void FSEventWrap::Initialize(Local<Object> target,
t->InstanceTemplate()->SetInternalFieldCount(1);
t->SetClassName(fsevent_string);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);
env->SetProtoMethod(t, "start", Start);
env->SetProtoMethod(t, "close", Close);

Expand Down
2 changes: 1 addition & 1 deletion src/js_stream.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ void JSStream::Initialize(Local<Object> target,
t->SetClassName(jsStreamString);
t->InstanceTemplate()->SetInternalFieldCount(1);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);

env->SetProtoMethod(t, "doAlloc", DoAlloc);
env->SetProtoMethod(t, "doRead", DoRead);
Expand Down
6 changes: 3 additions & 3 deletions src/node_crypto.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2729,7 +2729,7 @@ void Connection::Initialize(Environment* env, Local<Object> target) {
t->InstanceTemplate()->SetInternalFieldCount(1);
t->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "Connection"));

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);
env->SetProtoMethod(t, "encIn", Connection::EncIn);
env->SetProtoMethod(t, "clearOut", Connection::ClearOut);
env->SetProtoMethod(t, "clearIn", Connection::ClearIn);
Expand Down Expand Up @@ -6129,14 +6129,14 @@ void InitCrypto(Local<Object> target,

Local<FunctionTemplate> pb = FunctionTemplate::New(env->isolate());
pb->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "PBKDF2"));
env->SetProtoMethod(pb, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, pb);
Local<ObjectTemplate> pbt = pb->InstanceTemplate();
pbt->SetInternalFieldCount(1);
env->set_pbkdf2_constructor_template(pbt);

Local<FunctionTemplate> rb = FunctionTemplate::New(env->isolate());
rb->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "RandomBytes"));
env->SetProtoMethod(rb, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, rb);
Local<ObjectTemplate> rbt = rb->InstanceTemplate();
rbt->SetInternalFieldCount(1);
env->set_randombytes_constructor_template(rbt);
Expand Down
2 changes: 1 addition & 1 deletion src/node_file.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1480,7 +1480,7 @@ void InitFs(Local<Object> target,
Local<FunctionTemplate> fst =
FunctionTemplate::New(env->isolate(), NewFSReqWrap);
fst->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(fst, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, fst);
Local<String> wrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "FSReqWrap");
fst->SetClassName(wrapString);
Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ void Initialize(Local<Object> target,
env->NewFunctionTemplate(Http2Session::New);
session->SetClassName(http2SessionClassName);
session->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(session, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, session);
env->SetProtoMethod(session, "consume",
Http2Session::Consume);
env->SetProtoMethod(session, "destroy",
Expand Down
2 changes: 1 addition & 1 deletion src/node_http_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ void InitHttpParser(Local<Object> target,
#undef V
target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "methods"), methods);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);
env->SetProtoMethod(t, "close", Parser::Close);
env->SetProtoMethod(t, "execute", Parser::Execute);
env->SetProtoMethod(t, "finish", Parser::Finish);
Expand Down
2 changes: 1 addition & 1 deletion src/node_stat_watcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void StatWatcher::Initialize(Environment* env, Local<Object> target) {
FIXED_ONE_BYTE_STRING(env->isolate(), "StatWatcher");
t->SetClassName(statWatcherString);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);
env->SetProtoMethod(t, "start", StatWatcher::Start);
env->SetProtoMethod(t, "stop", StatWatcher::Stop);

Expand Down
2 changes: 1 addition & 1 deletion src/node_zlib.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ void InitZlib(Local<Object> target,

z->InstanceTemplate()->SetInternalFieldCount(1);

env->SetProtoMethod(z, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, z);
env->SetProtoMethod(z, "write", ZCtx::Write<true>);
env->SetProtoMethod(z, "writeSync", ZCtx::Write<false>);
env->SetProtoMethod(z, "init", ZCtx::Init);
Expand Down
4 changes: 2 additions & 2 deletions src/pipe_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ void PipeWrap::Initialize(Local<Object> target,
t->SetClassName(pipeString);
t->InstanceTemplate()->SetInternalFieldCount(1);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);

env->SetProtoMethod(t, "close", HandleWrap::Close);
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
Expand Down Expand Up @@ -104,7 +104,7 @@ void PipeWrap::Initialize(Local<Object> target,
};
auto cwt = FunctionTemplate::New(env->isolate(), constructor);
cwt->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(cwt, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, cwt);
Local<String> wrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "PipeConnectWrap");
cwt->SetClassName(wrapString);
Expand Down
2 changes: 1 addition & 1 deletion src/process_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ProcessWrap : public HandleWrap {
FIXED_ONE_BYTE_STRING(env->isolate(), "Process");
constructor->SetClassName(processString);

env->SetProtoMethod(constructor, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, constructor);

env->SetProtoMethod(constructor, "close", HandleWrap::Close);

Expand Down
2 changes: 1 addition & 1 deletion src/signal_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class SignalWrap : public HandleWrap {
FIXED_ONE_BYTE_STRING(env->isolate(), "Signal");
constructor->SetClassName(signalString);

env->SetProtoMethod(constructor, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, constructor);
env->SetProtoMethod(constructor, "close", HandleWrap::Close);
env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
env->SetProtoMethod(constructor, "unref", HandleWrap::Unref);
Expand Down
4 changes: 2 additions & 2 deletions src/stream_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void StreamWrap::Initialize(Local<Object> target,
Local<String> wrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "ShutdownWrap");
sw->SetClassName(wrapString);
env->SetProtoMethod(sw, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, sw);
target->Set(wrapString, sw->GetFunction());

Local<FunctionTemplate> ww =
Expand All @@ -79,7 +79,7 @@ void StreamWrap::Initialize(Local<Object> target,
Local<String> writeWrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "WriteWrap");
ww->SetClassName(writeWrapString);
env->SetProtoMethod(ww, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, ww);
target->Set(writeWrapString, ww->GetFunction());
env->set_write_wrap_constructor_function(ww->GetFunction());
}
Expand Down
5 changes: 2 additions & 3 deletions src/tcp_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ void TCPWrap::Initialize(Local<Object> target,
t->InstanceTemplate()->Set(env->onread_string(), Null(env->isolate()));
t->InstanceTemplate()->Set(env->onconnection_string(), Null(env->isolate()));

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
env->SetProtoMethod(t, "asyncReset", AsyncWrap::AsyncReset);
AsyncWrap::AddWrapMethods(env, t, AsyncWrap::kFlagHasReset);

env->SetProtoMethod(t, "close", HandleWrap::Close);

Expand Down Expand Up @@ -120,7 +119,7 @@ void TCPWrap::Initialize(Local<Object> target,
};
auto cwt = FunctionTemplate::New(env->isolate(), constructor);
cwt->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(cwt, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, cwt);
Local<String> wrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "TCPConnectWrap");
cwt->SetClassName(wrapString);
Expand Down
2 changes: 1 addition & 1 deletion src/timer_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TimerWrap : public HandleWrap {

env->SetTemplateMethod(constructor, "now", Now);

env->SetProtoMethod(constructor, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, constructor);

env->SetProtoMethod(constructor, "close", HandleWrap::Close);
env->SetProtoMethod(constructor, "ref", HandleWrap::Ref);
Expand Down
3 changes: 1 addition & 2 deletions src/tls_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -946,8 +946,7 @@ void TLSWrap::Initialize(Local<Object> target,
t->InstanceTemplate()->SetInternalFieldCount(1);
t->SetClassName(tlsWrapString);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
env->SetProtoMethod(t, "asyncReset", AsyncWrap::AsyncReset);
AsyncWrap::AddWrapMethods(env, t, AsyncWrap::kFlagHasReset);
env->SetProtoMethod(t, "receive", Receive);
env->SetProtoMethod(t, "start", Start);
env->SetProtoMethod(t, "setVerifyMode", SetVerifyMode);
Expand Down
2 changes: 1 addition & 1 deletion src/tty_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void TTYWrap::Initialize(Local<Object> target,
t->SetClassName(ttyString);
t->InstanceTemplate()->SetInternalFieldCount(1);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);

env->SetProtoMethod(t, "close", HandleWrap::Close);
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
Expand Down
4 changes: 2 additions & 2 deletions src/udp_wrap.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ void UDPWrap::Initialize(Local<Object> target,
env->SetProtoMethod(t, "unref", HandleWrap::Unref);
env->SetProtoMethod(t, "hasRef", HandleWrap::HasRef);

env->SetProtoMethod(t, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, t);

target->Set(udpString, t->GetFunction());
env->set_udp_constructor_function(t->GetFunction());
Expand All @@ -148,7 +148,7 @@ void UDPWrap::Initialize(Local<Object> target,
Local<FunctionTemplate> swt =
FunctionTemplate::New(env->isolate(), NewSendWrap);
swt->InstanceTemplate()->SetInternalFieldCount(1);
env->SetProtoMethod(swt, "getAsyncId", AsyncWrap::GetAsyncId);
AsyncWrap::AddWrapMethods(env, swt);
Local<String> sendWrapString =
FIXED_ONE_BYTE_STRING(env->isolate(), "SendWrap");
swt->SetClassName(sendWrapString);
Expand Down