Skip to content

Commit fd6a2b4

Browse files
committed
Additional changes from review
1 parent f3aa955 commit fd6a2b4

File tree

7 files changed

+21
-29
lines changed

7 files changed

+21
-29
lines changed

doc/threadsafe.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,14 @@ The removal of the dynamic call functionality has the following implications:
9999
ability to handle any necessary cleanup of the item's data.
100100
- The callback _does_ receive the context as a parameter, so a call to
101101
`GetContext()` is _not_ necessary. This context type is specified as the
102-
**first type argument** specified to `::New`, ensuring type safety.
102+
**first template argument** specified to `::New`, ensuring type safety.
103103
- The `New()` constructor accepts the `CallJs` callback as the **second type
104104
argument**. The callback must be statically defined for the API to access it.
105105
This affords the ability to statically pass the context as the correct type
106106
across all methods.
107107
- Only one C++ data type may be specified to every call to `[Non]BlockingCall()`
108-
-- the **third type argument** specified to `::New`. Any "dynamic call data"
109-
must be implemented by the user.
108+
-- the **third template argument** specified to `::New`. Any "dynamic call
109+
data" must be implemented by the user.
110110

111111

112112
### Usage Suggestions

doc/threadsafe_function_ex.md

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ Napi::ThreadSafeFunctionEx<ContextType, DataType, Callback>::ThreadSafeFunctionE
4040

4141
Returns a non-empty `Napi::ThreadSafeFunctionEx` instance. To ensure the API
4242
statically handles the correct return type for `GetContext()` and
43-
`[Non]BlockingCall()`, pass the proper type arguments to
43+
`[Non]BlockingCall()`, pass the proper template arguments to
4444
`Napi::ThreadSafeFunctionEx`.
4545

4646
### New
@@ -90,14 +90,15 @@ Returns a non-empty `Napi::ThreadSafeFunctionEx` instance.
9090
Depending on the targetted `NAPI_VERSION`, the API has different implementations
9191
for `CallbackType callback`.
9292
93-
When targetting version 4, `CallbackType` is:
94-
- `const Function&`
95-
- skipped, in which case the API creates a new no-op `Function`
93+
When targetting version 4, `callback` may be:
94+
- of type `const Function&`
95+
- not provided as an parameter, in which case the API creates a new no-op
96+
`Function`
9697
97-
When targetting version 5+, `CallbackType` is:
98-
- `const Function&`
99-
- `std::nullptr_t`
100-
- skipped, in which case the API passes `std::nullptr`
98+
When targetting version 5+, `callback` may be:
99+
- of type `const Function&`
100+
- of type `std::nullptr_t`
101+
- not provided as an parameter, in which case the API passes `std::nullptr`
101102
102103
### Acquire
103104
@@ -170,13 +171,6 @@ napi_status Napi::ThreadSafeFunctionEx<ContextType, DataType, Callback>::NonBloc
170171

171172
- `[optional] data`: Data to pass to the callback which was passed to
172173
`ThreadSafeFunctionEx::New()`.
173-
- `[optional] callback`: C++ function that is invoked on the main thread. The
174-
callback receives the `ThreadSafeFunction`'s JavaScript callback function to
175-
call as an `Napi::Function` in its parameters and the `DataType*` data pointer
176-
(if provided). Must implement `void operator()(Napi::Env env, Function
177-
jsCallback, DataType* data)`, skipping `data` if not provided. It is not
178-
necessary to call into JavaScript via `MakeCallback()` because N-API runs
179-
`callback` in a context appropriate for callbacks.
180174

181175
Returns one of:
182176
- `napi_ok`: The call was successfully added to the queue.

napi-inl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4399,7 +4399,7 @@ inline void AsyncWorker::OnWorkComplete(Napi::Env /*env*/, napi_status status) {
43994399
// ThreadSafeFunctionEx<ContextType,DataType,CallJs> class
44004400
////////////////////////////////////////////////////////////////////////////////
44014401

4402-
// Starting with NAPI 4, the JavaScript function `func` parameter of
4402+
// Starting with NAPI 5, the JavaScript function `func` parameter of
44034403
// `napi_create_threadsafe_function` is optional.
44044404
#if NAPI_VERSION > 4
44054405
// static, with Callback [missing] Resource [missing] Finalizer [missing]

test/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ let testModules = [
4343
'object/set_property',
4444
'promise',
4545
'run_script',
46+
'threadsafe_function_ex',
4647
'threadsafe_function/threadsafe_function_ctx',
4748
'threadsafe_function/threadsafe_function_existing_tsfn',
4849
'threadsafe_function/threadsafe_function_ptr',

test/threadsafe_function_ex/README.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
# Napi::ThreadSafeFunctionEx tests
22

3-
|Spec|Test|Native|Node|Description|
4-
|----|---|---|---|---|
5-
|call
3+
TODO

test/threadsafe_function_ex/test/basic.cc

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,9 +263,6 @@ class TSFNWrap : public base {
263263

264264
napi_threadsafe_function napi_tsfn;
265265

266-
// A threadsafe function on N-API 4 still requires a callback function, so
267-
// this uses the `EmptyFunctionFactory` helper method to return a no-op
268-
// Function on N-API 5+.
269266
auto status = napi_create_threadsafe_function(
270267
info.Env(), info[0], nullptr, String::From(info.Env(), "Test"), 0, 1,
271268
nullptr, Finalizer, this, CallJs, &napi_tsfn);
@@ -349,8 +346,8 @@ namespace simple {
349346
using ContextType = std::nullptr_t;
350347

351348
// Full type of our ThreadSafeFunctionEx. We don't specify the `ContextType`
352-
// here (even though the _default_ for the type argument is `std::nullptr_t`) to
353-
// demonstrate construction with no type arguments.
349+
// here (even though the _default_ for the template argument is
350+
// `std::nullptr_t`) to demonstrate construction with no template arguments.
354351
using TSFN = ThreadSafeFunctionEx<>;
355352

356353
class TSFNWrap;
@@ -363,7 +360,9 @@ class TSFNWrap : public base {
363360

364361
auto env = info.Env();
365362
#if NAPI_VERSION == 4
366-
// A threadsafe function on N-API 4 still requires a callback function.
363+
// A threadsafe function on N-API 4 still requires a callback function, so
364+
// this uses the `EmptyFunctionFactory` helper method to return a no-op
365+
// Function on N-API 4.
367366
_tsfn = TSFN::New(
368367
env, // napi_env env,
369368
TSFN::EmptyFunctionFactory(

test/threadsafe_function_ex/test/basic.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ class BasicTest extends TestRunner {
140140
}
141141

142142
/**
143-
* A `ThreadSafeFunctionEx<>` can be constructed with no type arguments.
143+
* A `ThreadSafeFunctionEx<>` can be constructed with no template arguments.
144144
* - Creates a threadsafe function with no context or callback or callJs.
145145
* - The node-addon-api 'no callback' feature is implemented by passing either
146146
* a no-op `Function` on N-API 4 or `std::nullptr` on N-API 5+ to the

0 commit comments

Comments
 (0)