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.

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

N api dataview #14382

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
85 changes: 85 additions & 0 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,41 @@ JavaScript TypedArray Objects are described in
[Section 22.2](https://tc39.github.io/ecma262/#sec-typedarray-objects)
of the ECMAScript Language Specification.


#### *napi_create_dataview*
<!-- YAML
added: REPLACEME
-->

```C
Copy link
Member

Choose a reason for hiding this comment

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

Can you leave a newline before this?

napi_status napi_create_dataview(napi_env env,
size_t byte_length,
napi_value arraybuffer,
size_t byte_offset,
napi_value* result)

```

- `[in] env`: The environment that the API is invoked under.
- `[in] length`: Number of elements in the DataView.
- `[in] arraybuffer`: ArrayBuffer underlying the DataView.
- `[in] byte_offset`: The byte offset within the ArrayBuffer from which to
start projecting the DataView.
- `[out] result`: A `napi_value` representing a JavaScript DataView.

Returns `napi_ok` if the API succeeded.

This API creates a JavaScript DataView object over an existing ArrayBuffer.
Dataview objects provide an array-like view over an underlying data buffer,
Copy link
Member

Choose a reason for hiding this comment

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

Capitalization: Dataview

but one which allows items of different size and type in the ArrayBuffer.

It's required that (byte_length * size_of_element) + byte_offset
Copy link
Member

Choose a reason for hiding this comment

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

size_of_element is always 1 for DataView. Delete the * size_of_element part plesse

should be <= the size in bytes of the array passed in. If not, a RangeError
Copy link
Member

Choose a reason for hiding this comment

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

"It is required that ... should be" does not make sense in my head (I am not a native English speaker though).

exception is raised.

Copy link
Member

Choose a reason for hiding this comment

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

napi_create_typedarray() has

It's required that (length * size_of_element) + byte_offset should be <= the size in bytes of the array passed in. If not, a RangeError exception is raised.

There should one here as well, with "(length * size_of_element)" replaced with byte_length.

JavaScript DataView Objects are described in
[Section 24.3][] of the ECMAScript Language Specification.

### Functions to convert from C types to N-API
#### *napi_create_number*
<!-- YAML
Expand Down Expand Up @@ -1552,6 +1587,36 @@ This API returns various properties of a typed array.
*Warning*: Use caution while using this API since the underlying data buffer
is managed by the VM



#### *napi_get_dataview_info*
<!-- YAML
added: REPLACEME
-->

```C
napi_status napi_get_dataview_info(napi_env env,
Copy link
Member

Choose a reason for hiding this comment

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

same comments here

napi_value dataview,
size_t* byte_length,
void** data,
napi_value* arraybuffer,
size_t* byte_offset)
```

- `[in] env`: The environment that the API is invoked under.
- `[in] dataview`: `napi_value` representing the DataView whose
properties to query.
- `[out] byte_length`: Number of bytes in the DataView.
- `[out] data`: The data buffer underlying the DataView.
- `[out] arraybuffer`: ArrayBuffer underlying the DataView.
- `[out] byte_offset`: The byte offset within the data buffer from which
to start projecting the DataView.

Returns `napi_ok` if the API succeeded.

This API returns various properties of a DataView.


#### *napi_get_value_bool*
<!-- YAML
added: v8.0.0
Expand Down Expand Up @@ -2019,6 +2084,25 @@ Returns `napi_ok` if the API succeeded.

This API checks if the Object passsed in is a typed array.



### *napi_is_dataview*
<!-- YAML
added: REPLACEME
-->

```C
Copy link
Member

Choose a reason for hiding this comment

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

(ditto)

napi_status napi_is_dataview(napi_env env, napi_value value, bool* result)
```

- `[in] env`: The environment that the API is invoked under.
- `[in] value`: The JavaScript value to check.
- `[out] result`: Whether the given `napi_value` represents a DataView.

Returns `napi_ok` if the API succeeded.

This API checks if the Object passsed in is a DataView.
Copy link
Member

Choose a reason for hiding this comment

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

Spelling: passsed.


### *napi_strict_equals*
<!-- YAML
added: v8.0.0
Expand Down Expand Up @@ -3165,6 +3249,7 @@ support it:
[Object Wrap]: #n_api_object_wrap
[Section 9.1.6]: https://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-defineownproperty-p-desc
[Section 12.5.5]: https://tc39.github.io/ecma262/#sec-typeof-operator
[Section 24.3]: https://tc39.github.io/ecma262/#sec-dataview-objects
[Working with JavaScript Functions]: #n_api_working_with_javascript_functions
[Working with JavaScript Properties]: #n_api_working_with_javascript_properties
[Working with JavaScript Values]: #n_api_working_with_javascript_values
Expand Down
67 changes: 67 additions & 0 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2957,6 +2957,73 @@ napi_status napi_get_typedarray_info(napi_env env,
return napi_clear_last_error(env);
}

napi_status napi_create_dataview(napi_env env,
size_t byte_length,
napi_value arraybuffer,
size_t byte_offset,
napi_value* result) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, arraybuffer);
CHECK_ARG(env, result);

v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);
RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg);

v8::Local<v8::ArrayBuffer> buffer = value.As<v8::ArrayBuffer>();
v8::Local<v8::DataView> DataView = v8::DataView::New(buffer, byte_offset,
byte_length);

*result = v8impl::JsValueFromV8LocalValue(DataView);
return GET_RETURN_STATUS(env);
}

napi_status napi_is_dataview(napi_env env, napi_value value, bool* result) {
CHECK_ENV(env);
CHECK_ARG(env, value);
CHECK_ARG(env, result);

v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
*result = val->IsDataView();

return napi_clear_last_error(env);
}

napi_status napi_get_dataview_info(napi_env env,
napi_value dataview,
size_t* byte_length,
void** data,
napi_value* arraybuffer,
size_t* byte_offset) {
CHECK_ENV(env);
CHECK_ARG(env, dataview);

v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(dataview);
RETURN_STATUS_IF_FALSE(env, value->IsDataView(), napi_invalid_arg);

v8::Local<v8::DataView> array = value.As<v8::DataView>();


Copy link
Member

Choose a reason for hiding this comment

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

Nit: I would try to avoid multiple consecutive empty lines within functions. It usually does not improve readability.

if (byte_length != nullptr) {
*byte_length = array->ByteLength();
}

v8::Local<v8::ArrayBuffer> buffer = array->Buffer();
if (data != nullptr) {
*data = static_cast<uint8_t*>(buffer->GetContents().Data()) +
array->ByteOffset();
}

if (arraybuffer != nullptr) {
*arraybuffer = v8impl::JsValueFromV8LocalValue(buffer);
}

if (byte_offset != nullptr) {
*byte_offset = array->ByteOffset();
}

return napi_clear_last_error(env);
}

napi_status napi_get_version(napi_env env, uint32_t* result) {
CHECK_ENV(env);
CHECK_ARG(env, result);
Expand Down
15 changes: 15 additions & 0 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,21 @@ NAPI_EXTERN napi_status napi_get_typedarray_info(napi_env env,
napi_value* arraybuffer,
size_t* byte_offset);

NAPI_EXTERN napi_status napi_create_dataview(napi_env env,
size_t length,
napi_value arraybuffer,
size_t byte_offset,
napi_value* result);
NAPI_EXTERN napi_status napi_is_dataview(napi_env env,
napi_value value,
bool* result);
NAPI_EXTERN napi_status napi_get_dataview_info(napi_env env,
napi_value dataview,
size_t* bytelength,
void** data,
napi_value* arraybuffer,
size_t* byte_offset);

// Methods to manage simple async operations
NAPI_EXTERN
napi_status napi_create_async_work(napi_env env,
Expand Down
8 changes: 8 additions & 0 deletions test/addons-napi/test_dataview/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"targets": [
{
"target_name": "test_dataview",
"sources": [ "test_dataview.c" ]
}
]
}
14 changes: 14 additions & 0 deletions test/addons-napi/test_dataview/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use strict';
const common = require('../../common');
const assert = require('assert');

// Testing api calls for arrays
const test_dataview = require(`./build/${common.buildType}/test_dataview`);

//create dataview
const buffer = new ArrayBuffer(128);
const template = Reflect.construct(DataView, [buffer]);

const theDataview = test_dataview.CreateDataView(template);
assert.ok(theDataview instanceof DataView,
'The new variable should be of type Dataview');
48 changes: 48 additions & 0 deletions test/addons-napi/test_dataview/test_dataview.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#include <node_api.h>
#include <string.h>
#include "../common.h"

napi_value CreateDataView(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args [1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

NAPI_ASSERT(env, argc == 1, "Wrong number of arguments");

napi_valuetype valuetype;
napi_value input_dataview = args[0];

NAPI_CALL(env, napi_typeof(env, input_dataview, &valuetype));
NAPI_ASSERT(env, valuetype == napi_object,
"Wrong type of argments. Expects a dataview as the first"
Copy link
Member

Choose a reason for hiding this comment

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

Spelling: argments, and an additional space between dataview and as.
As dataview refers to a type here, I think DataView would be better.

Copy link
Member

Choose a reason for hiding this comment

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

Also, there is no space between first and argument..

"argument.");
Copy link
Member

Choose a reason for hiding this comment

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

Reminder to whoever lands this: extra space in front.


bool is_dataview;
NAPI_CALL(env, napi_is_dataview(env, input_dataview, &is_dataview));
NAPI_ASSERT(env,is_dataview,
"Wrong type of arugments. Expects a dataview as first argument.");
Copy link
Member

Choose a reason for hiding this comment

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

Spelling: arugments.
As dataview refers to a type here, I think DataView would be better.

Copy link
Member

@tniessen tniessen Jul 26, 2017

Choose a reason for hiding this comment

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

Also, this could just be the same message as above.

size_t byte_offset = 0;
size_t length = 0;
napi_value buffer;
NAPI_CALL(env,
napi_get_dataview_info(env, input_dataview, &length, NULL,
&buffer, &byte_offset));

napi_value output_dataview;
NAPI_CALL(env,
napi_create_dataview(env, length, buffer,
byte_offset, &output_dataview));

return output_dataview;
}

void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_property_descriptor descriptors[] = {
DECLARE_NAPI_PROPERTY("CreateDataView", CreateDataView)
};

NAPI_CALL_RETURN_VOID(env,napi_define_properties(
Copy link
Member

Choose a reason for hiding this comment

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

Missing space after env,.

env, exports,sizeof(descriptors) / sizeof(*descriptors), descriptors));
Copy link
Member

Choose a reason for hiding this comment

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

Missing space after exports,.

}

NAPI_MODULE(addon, Init)